Transfer Assignment

[jon_m]

Hi again @gaio,

I’ve just noticed that your questions and discussion with @thecil for this assignment are related to your other questions in the Payable Functions discussion topic, which I’ve answered for you there. Hopefully, you can now see the key difference between…

1. Transfers of ether/wei value between the Bank contract address and external (wallet) addresses; and
2. “Internal accounting adjustments” to the separate Bank account holder balances within the Bank contract (recorded in the mapping), which do not involve any actual movement of ether/wei between Ethereum addresses, but instead record the changes in the entitlement each Bank account holder has to withdraw ether from the Bank contract.

@gaio

And if the transfer method is fundamental to transferring between any 2 addresses, why is it not involved the the deposit function?

Once you’ve understood the key difference I’ve described above, you should be able to see that the transfer function is different to the deposit and withdraw functions, in that it doesn’t involve any ether/wei entering or leaving the Bank contract. It handles internal transfers of funds between existing Bank account holders. So, the net effect to the contract balance is zero. However, the contract still needs to adjust the amount each of the parties involved in the internal transfer is entitled to withdraw from the contract (their share of the total pooled funds) i.e. increase the receiver’s entitlement (balance) by the same amount the sender’s entitlement is reduced, with the code…

balance[from] -= amount;
balance[to] += amount;

Just let me know if you still have any uncertainties or questions about any of these points

[chen]

pragma solidity 0.7.5;
contract Bank {
    
    mapping(address => uint) balances;
    
    //modifier checkBalance{
    //    require(amount >= balances[msg.sender]);
     //   _;
    //}
    event Log(uint amount,address indexed to );
    function Deposit() public payable {
        emit Log(msg.value,msg.sender);
        balances[msg.sender] += msg.value;
    }
    
    function withdraw(uint amount) public {
        require(balances[msg.sender] >= amount);
        msg.sender.transfer(amount);
    }
}

[chen]

there is no way to pass amount arg to modifier

[jon_m]

Hi @chen,

The require statement you’ve added is correct, but your withdraw function is missing an important line of code…

If you deploy your contract, make a deposit, and then call your withdraw function as it is, the caller’s external wallet address receives the requested withdrawal amount, but their balance in the mapping is not reduced (you can add a getBalance function to check this). 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 that this is a very serious bug.

msg.sender.transfer(amount) transfers ether from the contract address balance to the caller’s external wallet 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.

---

Events should only be emitted once all of the associated operations have been completed. This can only be assured if the emit statement is positioned at the end of the function body (but before the return statement, if there is one). Have a look again at where you’ve positioned your emit statement in the deposit() function…

---

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.

You are 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 this can be done by including the getBalance function, which I mentioned above: getBalance() can be called separately to consult the individual account holder’s updated balance after the withdrawal.

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

[jon_m]

@chen

there is no way to pass amount arg to modifier

Yes, there is…

// Add an amount parameter to the modifier's header...
modifier checkBalance(uint amount) {
    // require(amount >= balances[msg.sender]);
    // Your condition in the require statement was the wrong way round
    require(balances[msg.sender] >= amount, "Insufficient balance");
    _;
}

// In the withdraw function header, call the modifier with the amount argument...
function withdraw(uint amount) public checkBalance(amount) { ... }

[chen]

@jon_m
As seen, we have two amount variable passed to the function.
What if I pass different amount value into the function?
function withdraw(uint amount) public checkBalance(amount) { ... }

[chen]

@jon_m

    function withdraw(uint amount) public {
        require(balances[msg.sender] >= amount);
        msg.sender.transfer(amount);
    }

Hi Jon,
In this function, who is sender and who is recipient?
This block of code looks confusing:

require(balances[msg.sender] >= amount); msg.sender looks like the sender,
while msg.sender.transfer(amount); implies msg.sender is a recipient .

[alexdemc]

The first line checks the balance of msg.sender to make sure that they have enough Ethereum deposited to call the .transfer function.

Think of the line of code as “msg.sender will transfer amount”.

EDIT: if there was a recipient parameter, then the recipient address would receive the transferred Ether. Instead msg.sender is transferring the funds from the contract back to their address.

[chen]

Ok.
Then who is sender and who is recipient in withdraw function???

[jon_m]

@chen

As seen, we have two amount variable passed to the function.

No … you only have one parameter (one uint value passed to the withdraw function):

function withdraw(uint amount) ...

A function’s parameters (the values input into the function) are defined between the parentheses that come immediately after the function name.

The call to the checkBalance modifier references the amount parameter, and so it calls the modifier with the same uint value that has been input into the withdraw function. Notice the difference between:

uint amount 
/* DEFINES the value type as an unsigned integer
   and assigns it to an identifier/name (amount)
   (e.g. for a value input into a function or assigned to a variable) */

amount
// REFERENCES the value that has been assigned to this identifier (amount) 

[alexdemc]

msg.sender sends the transaction to the contract, the contract transfers the Ether and msg.sender is the recipient.

[jon_m]

Hi @chen,

@chen

require(balances[msg.sender] >= amount); msg.sender looks like the sender,
while msg.sender.transfer(amount); implies msg.sender is a recipient .

msg.sender is used in the body of a Solidity function to reference the address that calls this function. We may need to reference the caller’s address for very different reasons within the same function:

(1) Each user’s share of the total amount of ether held in the contract is recorded and tracked in the mapping. This is our internal accounting mechanism, and each individual user is identified by their external address (the address they are depositing ether into the contract from, and withdrawing ether out of the contract to). The mapping doesn’t hold ether: it records each individual user’s entitlement to withdraw ether from the Bank smart contract.

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

So, as @alexdemc has explained …

@alexdemc

The first line checks the balance of msg.sender to make sure that they have enough Ethereum deposited to call the .transfer function.

Notice that it doesn’t check the total contract balance. It checks whether the address of the function caller is entitled to withdraw their requested amount from the smart contract. In order for the mapping to be used in this way, it is essential that …

@jon_m

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.

(2)

@chen

msg.sender.transfer(amount); implies msg.sender is a recipient .

Correct

@alexdemc

msg.sender sends the transaction to the contract, the contract transfers the Ether and msg.sender is the recipient.

Or in other words… the address calling the withdraw function is requesting that the Bank contract transfers the ether amount from the Bank contract address to their own external address (let’s say their wallet).

And as @alexdemc has also said …

@alexdemc

if there was a recipient parameter, then the recipient address [could] receive the transferred Ether.

e.g.

function withdraw(uint amount, address recipient) public {
    require(balances[msg.sender] >= amount);
    balances[msg.sender] -= amount;
    payable(recipient).transfer(amount);
}

Here, the function caller (msg.sender) is requesting that the Bank contract sends the ether amount from the Bank contract to the external address input as the recipient argument. But notice that the amount has to be checked against, and deducted from, the msg.sender's individual account balance in the mapping, because they are effectively transferring ether from their individual share of the total contract balance to the recipient.

The transfer function is different to the deposit and withdraw functions, in that it doesn’t involve any ether entering or leaving the Bank contract. It handles internal transfers of funds between existing Bank account holders. So, the net effect to the contract balance is zero. However, the contract still needs to adjust the amount each of the parties involved in the internal transfer is entitled to withdraw from the contract (their share of the total pooled funds) i.e. increase the receiver’s entitlement (balance) by the same amount the sender’s entitlement is reduced, with the code…

balance[from] -= amount;
balance[to] += amount;

I hope things are gradually becoming clearer

[chen]

okay. I’m not sure I fully understand msg.sender. It looks more complicated.

So far , we have two addresses : Bank contract address and external address
What confuses me is that whether these two msg.sender in line 1 and 2 has the same value?

require(balance[msg.sender] >= amount);   line 1
msg.sneder.transer(amount);                       line 2

what address value does msg.sender has in line 1 and 2 ?
is it bank contract address or external address?

[chen]

@jon_m
function withdraw(uint amount) public checkBalance(amount) { ... }

for example, 10 is passed to checkBalance(amount) but user passed 20 to withdraw function .
That makes no sense. The amount must be same in both withdraw and checkBalance.
How do I make amount in checkBalance consistent with user’s input?

[jon_m]

@chen

What confuses me is that whether these two msg.sender in line 1 and 2 has the same value?

Yes… it has the same value in both lines of code.

@jon_m

msg.sender is used in the body of a Solidity function to reference the address that calls this function.

In Remix, the address that calls a function is the one selected in the Account field at the top of the Deploy & Run Transactions panel. You can change this address to simulate multiple users, each with a separate external wallet address.

@chen

what address value does msg.sender has in line 1 and 2 ?
is it bank contract address or external address?

So, in the withdraw function it’s the external address of the user who has funds in the Bank contract and wants to withdraw some (or all) of them.

[jon_m]

If the user calls the withdraw function with 10 wei, then the amount parameter will be assigned the value of 10 wei, and so the checkBalance modifier will be called with that same amount.

@chen

The amount must be same in both withdraw and checkBalance.

Correct

@chen

How do I make amount in checkBalance consistent with user’s input?

You are making it consistent by calling it with amount. This amount REFERENCES the amount input by the user into the withdraw function. The user doesn’t call the modifier, the function calls the modifier (before executing the code in its function body) with the amount passed to it by the user.

[chen]

clear…
Thanks

[jon_m]

[moragarcia75]

pragma solidity 0.7.5;

contract Bank{
mapping (address=>uint)balance;

address owner;

event depositDone (uint amount, address depositedTo);
event balanceTransferred (uint amount, address sender,address recipient);

modifier onlyOwner{
    require (msg.sender==owner);
    _;
}

constructor(){
    owner=msg.sender;
}

function deposit ()public payable returns (uint) {  //Adding "payable" allows the smart contract to receive ETH
    balance[msg.sender]+= msg.value; //This keeps track of each address so we know who deposited ETH.
    emit depositDone(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, "balance not enough");
    msg.sender.transfer(amount);
}

    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;
    emit balanceTransferred (amount, from, to);
}

}

[jon_m]

Hi @moragarcia75,

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 wallet 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 wallet 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. You’ve added a good comment about the importance this in your deposit() function …

@moragarcia75

balance[msg.sender]+= msg.value; //This keeps track of each address so we know who deposited ETH.

… 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, as well, that we’ve made similar adjustments to both the sender’s and the recipient’s balances in the mapping in the _transfer() function: the sender’s balance is reduced by the same amount that the recipient’s balance is increased by.

---

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 a yellow/orange warning about this.

… and don’t forget about how to improve the positioning of your transfer event’s emit statement, which I’ve explained here.

---

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.

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