Here’s my solution
require to check withdraw feasibilitywithdrawn
event withdrawn(uint amount, address _address);
function withdraw(uint amount) public returns (uint) {
require(balance[msg.sender] >= amount, "Insufficient funds");
payable (msg.sender).transfer(amount);
balance[msg.sender] -= amount;
emit withdrawn(amount, msg.sender);
}
Hi All,
I have some troubles with github code of the solution.
Here are my steps, and at the end of this post the official Github code (+ 2 variables for knowing contract address & balance, but not still used here) :
At this point, I’ve got an error saying “Balance not sufficient”, and here are my decoded input :
{
“address recipient”: “0xCA35[…]”,
“uint256 amount”: {
“_hex”: “0x01”,
“_isBigNumber”: true
}
}
Final checking : my msg.sender balance is not equals to 10000000000000000000 or less, but it equals to zero…
Don’t understand at all this behaviour.
PS : I am sure that this official code worked well, but I came back on this course of ETH101 in order to make some tests, like discussed with @jon_m, maybe I have some cache to clear in Remix, or others…
Many thanks for your thoughts.
Original code but it’s the same that GitHub Code :
https://github.com/filipmartinsson/solidity-0.7.5/blob/main/transfer-assignment-solution.sol
pragma solidity 0.7.5;
contract Bank {
mapping(address => uint) balance;
address public contractAddress = address(this);
uint256 public contractBalance = address(this).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);
balance[msg.sender] -= amount;
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;
}
}
Hi @11lll,
You have added both of the essential lines of code needed to solve the problem with the withdraw function 
Your withdrawn event declaration and corresponding emit statement are also both well coded. The emit statement is in the correct position in the withdraw() function body and will log appropriate data when the withdraw() function is successfully executed.
However, 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 you’ve included it in your function header, you should also include a return statement in your function body. You should have got a yellow/orange compiler warning about this.
Also, have a look at this post which explains the importance of the order of the statements within your withdraw function body.
Just let me know if you have any questions 
Hi @_Vincent,
This is great that you’ve come back to experiment and test the code in detail. You’ll learn a lot this way.
Yes, it will work.
These are well coded, and Solidity will automatically create getters for both state variables, which you can call from Remix to retrieve the contract address and the contract balance
Not sure what you mean by “null value”…?
In the contract code there is no constructor assigning the deployer’s address (msg.sender) to a state variable (e.g. owner). So for the purposes of transacting with this particular contract, the deployer’s address is irrelevant.
If you are calling the transfer function, then you are right to copy one of the addresses from the Account field dropdown for the recipient argument. The uint value you enter for the amount argument is in wei not ether. So, to transfer an amount in ether you need to add 18 zeros! It is annoying but that’s the way it is I’m afraid. We have to do it like this for the amount arguments when we call the transfer() function and the withdraw() function. Solidity can only process integers (i.e.whole numbers with no decimals) so all ether values are in units of wei. When an address sends ether to the deposit() function, Remix gives us the option of entering the value in wei, gwei, finney or ether. This is just for convenience; the actual value sent to the function (and referenced by msg.value) will always be converted to units of wei — in other words, if you enter 1 in the Value field and set the adjacent dropdown to ether, Remix will add on the 18 zeros for you when this value is sent to the function 
After you’ve copied and pasted an address for the recipient argument, you need to make sure that you change the address displayed in the Account field to one which already has funds deposited in the Bank contract and therefore has a positive individual balance recorded in the mapping. When you click the transfer button, the address displayed in the Account field is the msg.sender address which calls the transfer() function and transfers an amount in wei from its own individual balance (its share of the total contract balance) to the recipient’s individual balance. If the displayed address has a zero balance in the Bank contract, this is probably why your transfer() function is reverting and throwing the error “Balance not sufficient”.
I don’t think this will be the problem.
I hope this helps to resolve things, but let me know if you still experience problems, or if you have any further questions.
Hi @jon_m, many thanks for your time !
Indeed, after perform some tests, I can confirm this is wei/ether the “issue”, but it’s amazing to see that with transfer() function here, a value of “2” is well transfered, but “1” give the error “Balance not sufficient”.
It helps me a lot too to check the documentation of https://docs.soliditylang.org/en/v0.8.10/index.html and https://remix-ide.readthedocs.io/en/latest/index.html. I encourage everyone to do the same in addition. Courses very well suited, but some details are in official docs.
A last trouble from my own : the contract balance (address(this).balance) is initialized at the deployment of contract at “1 ETH” and it’s impossible by code to update this amount.
At the end, If then, I use deposit function for put 1 ETH in the msg.value of msg.sender, it works.
But, in my mind, I thought, once this msg.value is increased at 1ETH, then the contract balance will be equals to 2 ETH, but it’s not the case.
I think at this level, we have to manage a library with the good mapping of all users belonging to the contract and see if the sum of all balances really equals to the adress(this).balance of our contract.
Hi @_Vincent,
Sorry, I misinformed you when I said…
As you say, both of the values for these 2 state variables are set on deployment. This means that the contractAddress getter will correctly return the contract address because this never changes. However, contractBalance will be set and remain at 0, because address(this).balance is only evaluated on deployment. In order to return the contract balance at specific points in time, you need to add the function that I gave you before, which contains address(this).balance in its local scope. This piece of code will then only be evaluated whenever this function is called…
With the code you have at the moment, the contractBalance getter should always return 0 wei (the contract balance on deployment), not the 1000000000000000000 wei (1 ETH) you say it’s returning when you call it… ?
Anyway, if you replace the contractBalance state variable with the function above, you should now be able to do what you say here …
It’s great you’re starting to use and check things in the Solidity documentation. It can take a while to get used to, and it can be difficult to read and understand at first, but with patience and practice you’ll find it a valuable resource.
This shouldn’t be happening. If you check just before transferring 1 ETH (by calling getBalance) that the caller of the transfer() function (msg.sender) has an individual balance in the mapping of at least 1000000000000000000 wei (1 ETH) after any previous transactions, then require() shouldn’t fail and throw the error messge “Balance not sufficient”. Are you sure you’re inputting a transfer amount of 1 + 18 zeros, and no more? It’s easy to make a mistake with the zeros, and if you add 19 instead of 18, it will be trying to transfer 10 ETH instead 1 ETH.
Let me know how you get on resolving these issues.
Hi @jon_m,
I made a video on the error, so maybe you’ll understand what happens : https://www.youtube.com/watch?v=r1UKk8aOmwg
This is very strange, because the same actions worked well, unless I made a mistake.
I perform just a transfer, with the right number of zero. And no “payable” term to add in the official source code for transfer() function (I write that, because sometimes error message caught and it relates a missing payable term).
Thanks,
Vincent.
Hi @_Vincent,
Can you post an exact copy of the full code in the contract file you are deploying and interacting with? I also want to deploy and test your actual code myself
And just to confirm, you should be doing and seeing something like the following…
(I’m going to use 2 different addresses from the Account field dropdown — let’s call them A and B)
You can then try the same thing with different amounts, and then different transaction combinations.
Hi @jon_m,
many thanks for your process.
Unfortunately, I stuck at the step 5, with following output :
"Note: The called function should be payable if you send value and the value you send should be less than your current balance."
Debug infos : "Transaction mined but execution failed"
Here is my code :
OWNABLE.SOL :
pragma solidity 0.7.5;
contract Ownable
{
address payable public owner;
address public contractAddress;
uint256 public contractBalance;
constructor() payable
{
owner = msg.sender;
//contractBalance = address(this).balance;
contractAddress = address(this);
}
//MODIFIERS
modifier onlyOwner()
{
require(msg.sender == owner);
_;
}
}
DESTROYABLE.SOL :
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable
{
function close() public onlyOwner
{
selfdestruct(owner);
}
}
BANK.SOL :
pragma solidity 0.7.5;
import "./Destroyable.sol";
interface GovernmentInterface{
function addTransaction(address _from, address _to, uint _amount) external;
}
contract Bank is Destroyable
{
GovernmentInterface govInstance = GovernmentInterface(0xd9145CCE52D386f254917e481eB44e9943F39138);
constructor() payable {
}
mapping(address => uint) balance;
//EVENTS
event balanceAdded(uint amount, address indexed depositedTo);
event depositDone(uint value, address indexed sender);
event closureDone(uint value, address indexed sender);
//BALANCE
function addBalance(uint _amount) public onlyOwner returns(uint)
{
balance[msg.sender] += _amount;
emit balanceAdded(_amount,msg.sender);
return balance[msg.sender];
}
function getBalance(address _address) public view returns(uint)
{
return balance[_address];
}
//address that originally deploys the contract
function getOwnerBalance() public view returns(uint)
{
return balance[msg.sender];
}
function getContractBalance() public view returns(uint) {
return address(this).balance;
}
//DEPOSIT
function depositToOwner() public payable returns(uint)
{
balance[msg.sender] += msg.value;
emit depositDone(msg.value, msg.sender);
return balance[msg.sender];
}
//WITHDRAW
function withdraw(uint amount) public payable onlyOwner returns(uint)
{
require(balance[msg.sender] >= amount);
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
return balance[msg.sender];
}
//TRANSFER
function transfer(address _recipient,uint _amount) public {
//check balance of msg.sender
require(balance[msg.sender] >= _amount, "Balance not sufficient");
require(_recipient != owner, "Don't transfer money to yourself");
uint previousBalance = balance[msg.sender];
_transfer(msg.sender, _recipient, _amount);
govInstance.addTransaction(msg.sender, _recipient, _amount);
assert(balance[msg.sender] == previousBalance - _amount);
}
function _transfer(address _from, address _to, uint _amount) private
{
balance[_from] -= _amount;
balance[_to] += _amount;
}
}My Solution:
pragma solidity 0.8.9;
contract Bank {
mapping(address => uint) balance;
address owner;
event depositDone(uint amount, address indexed depositedTo);
//event balanceTransferred(uint amount, address indexed depositedFrom, address indexed depositedTo);
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(balance[msg.sender]>=amount, "Balance not sufficient");
payable(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);
// emit balanceTransferred(amount, msg.sender, recipient);
assert(balance[msg.sender] == previousSenderBalance - amount);
}
function _transfer(address from, address to, uint amount) private {
balance[from] -= amount;
balance[to] += amount;
}
}
Hi @_Vincent,
Good job I asked to see your full code, because I didn’t realise you were including the external function call to the Government contract…
I think what’s probably causing your transfer() function to revert is not changing the Government contract’s address assigned to your govInstance in Bank, after you’ve deployed Government. Have a look at this post, which explains the issue and how to resolve it. It also explains why the error message you’re getting isn’t very helpful.
If that doesn’t work, post your Government contract as well, as that is part of your solution (you should be deploying it before Bank).
Hi @_Vincent,
Other comments and issues with your contracts…
Ownable
(1)
This code can be removed, because it serves no purpose. No value is assigned to it. Only the Bank contract needs to be deployed, because it inherits the necessary functionality from Destroyable and Ownable. So, the getContractBalance() function you’ve included in Bank is sufficient to retrieve the deployed Bank contract’s balance at any specific point in time.
(2) You don’t need to use the constructor to assign the contract address to the contractAddress state variable on deployment. The following line of code will do this more concisely…
address public contractAddress = address(this);
Because this state variable is inherited, address(this) will capture the address at which the derived contract (which inherits this state variable) is deployed.
(3)  The constructor doesn’t need to be marked payable
Destroyable  
Bank
(1) You don’t need another constructor in Bank.
(2) This getter …
… will return the balance of any address that calls it, and not just the contract owner’s balance. To allow anybody to check the contract owner’s balance, you would have to look up the owner 's balance in the mapping (not msg.sender 's) …
return balance[owner];
(3) The addBalance() function is not right in this contract, and should have been removed when the deposit() function was added. The addBalance() function was for adding non-ether amounts before we learnt how to send ether to a contract using a payable function. The increase to a user’s individual balance in the mapping, by the same amount as the ether sent to the function and deposited in the contract, is now being performed in the deposit() function. Having both functions allows the contract owner to inflate their individual balance in the mapping, by calling the addBalance() function, and then to withdraw more than their share of the total contract balance, effectively stealing other users’ funds.
The associated balanceAdded event and corresponding emit statement should, therefore, also be removed.
(4) The closureDone event is not implemented anywhere in the contract, and so it’s redundant code.
(5) 
Additional information
(6) By adding the onlyOwner modifier to the withdraw() function, only the contract owner can withdraw their funds. But the contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so it only seems fair that all users should be able to withdraw their funds as well, don’t you think? 
On this note, you should change the name of your deposit() function. At the moment it is depositToOwner(), which isn’t true. The function caller deposits funds to their own individual balance (not to the contract owner’s).
(7) 
Any address with funds deposited in the contract can make an internal transfer, so we want this require statement to prevent the function caller from transferring funds to themselves. But your condition doesn’t do that; it prevents any function caller from transferring funds to the contract owner.
(8)  If you want to end up with a complete contract, then you are missing the transfer event from the Events Assignment. I think you may have missed it. The task was to add code, so that each successful transfer logs an event with appropriate data.
Nice solution @skawal22
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.
Just let me know if you have any questions
@jon_m, Thanks again. And i appreciate your suggestion. I read the link post and now understand why you added suggestions there.
Hi @jon_m,
thanks for your steps.
Tried again and it works with some adjustments :
Step 5 resolved but removing government instance part (not necessary in my case, because this training happens before using governmentinstance in ETH courses).
Got issue with withdrawal and send transfer operation, but it’s logic and the issue coming from modifier onlyOwner.
This is not an issue, but we can’t withdrawal if we are not the owner, and we can’t perform a “reverse” transfer, cause to onlyOwner modifier.
Thanks again for your time @jon_m,
some feedbacks :
Ownable (3) I set the constructor with “payable”, because without that, we can’t deploy the contract with a value in “Value” fields. I notified it during my first tests.
Bank (6) Hum… it’s my new bank concept : only Owner can withdraw funds , you right ! I checked that, in my previous post with my tests 
Thanks for all your taughts, missing notions are now acquired
My solution for the assignment.
pragma solidity 0.8.9;
contract Bank {
mapping(address => uint) balance;
address owner;
event DepositDone(uint amount, address indexed depositedTo);
event TransferDone(uint amount, uint balance, address indexed transferedTo);
event WithdrawDone(uint amount, uint balance, address indexed withdrawnTo);
modifier onlyOwner {
require(msg.sender == owner, "You are not the 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];
}
function transfer(address _recipient, uint _amount) public {
require(balance[msg.sender] >= _amount, "Insufficient balance");
require(msg.sender != _recipient, "Don't send money to yourself.");
balance[msg.sender] -= _amount;
balance[_recipient] += _amount;
payable(_recipient).transfer(_amount);
emit TransferDone(_amount, balance[msg.sender], _recipient);
}
function withdraw(uint _amount) public {
require(balance[msg.sender] >= _amount, "Insufficient balance");
balance[msg.sender] -= _amount;
payable(msg.sender).transfer(_amount);
emit WithdrawDone(_amount, balance[msg.sender], msg.sender);
}
}
pragma solidity 0.7.5;
contract Bank{
mapping(address => uint) balance;
address owner;
event depositeDone(uint amount, address indexed depositedTo);
event withdrawDone(uint amount, address indexed withdraw);
modifier onlyOwner {
msg.sender == owner;
_;
}
constructor(){
owner = msg.sender;
}
function deposit() public payable returns(uint){
balance[msg.sender] += msg.value;
emit depositeDone(msg.value, msg.sender);
return balance[msg.sender];
}
function withdraw(uint amount) public{
require(amount <= balance[msg.sender], "You haven't sufficient balance");
msg.sender.transfer(amount);
emit withdrawDone(amount, msg.sender);
balance[msg.sender] -= amount;
}
function getBalance() public view returns(uint){
return balance[msg.sender];
}
function transferTo(address recipient, uint amount) public {
require(balance[msg.sender] >= amount, "Balance not sufficient");
require(msg.sender != recipient, "Don't send 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;
}
}
Hi @_Vincent,
Yes, it will be easier to get it all working first without the extra complication of the Government instance and external function call. You can add this in afterwards.
You should remove onlyOwner from the withdraw function header for the following reason …
Do you mean the transfer of ether out of the contract to the external address, which happens as part of the withdraw transaction, i.e. msg.sender.transfer(amount) ?
By “reverse” transfer do you mean step 15? …
There is no reason why this shouldn’t work, if your first transfer now works in step 5. The onlyOwner modifier isn’t applied to the transfer() function. Make sure you have Address A in the recipient argument field, and Address B showing in the Account field, before calling the function. Or is the issue something else?
Hello @jon_m,
Yes, it was not possible for I doing a transfer as a non Owner, but logic regarding the onlyOwner modifier.
Exactly, even it’s not the right term to use “reverse”.
Issue something else finally, but to be honest I don’t remember cause I’m working on the final project for ETH 101.
Sure, it was a little trick :).
Many thanks Jon