Hi @david.maluenda,
I can’t see anything in your Bank contract that could be causing the transfer() function to revert. The most common cause of this issue is not changing the Government contract’s address, which is assigned to your governmentInstance. Have a look at this post, which explains the issue and how to resolve it. It also explains why the error message you got isn’t very helpful.
Let me know if that doesn’t work, and I’ll take a look at your Government contract as well.
Having looked at your Bank contract, I’ve also noticed a couple of other things (unrelated to the error message) …
(1) 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. 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.
Once you’ve modified your withdraw function for this, have a look at this post which explains the importance of the order of the statements within the function body.
(2) Your transfer event and corresponding emit statement are both well coded, and the emit statement will log appropriate data when the transfer() function is successfully executed.
A couple of comments…
You also seem to have missed out the Inheritance Assignment. I would definitely go back and complete that before you finish this course and move on to 201, just to make sure you are comfortable with how inheritance works and how to code for it.
Let me know if anything is unclear, or if you have any questions about any of these additional points 
Thanks for your answer mate, forgot about the assignment will go back and fix it.
I’m not entirely sure what I’m doing wrong here with this value call. I keep getting an error on the Transfer function from Bank Contract:
The transaction has been reverted to the initial state.
Note: The called function should be payable if you send value and the value you send should be less than your current balance.
Can someone offer some advice?
pragma solidity 0.7.5;
contract Government {
struct Transaction{
address from;
address to;
uint amount;
uint txID;
}
Transaction[] transactionLog;
function addTransaction(address _from, address _to, uint _amount) external payable {
transactionLog.push(Transaction(_from, _to, _amount, transactionLog.length));
}
function getTransaction(uint _index) public view returns(address, address, uint) {
return (transactionLog[_index].from, transactionLog[_index].to, transactionLog[_index].amount);
}
function getBalance() external view returns(uint) {
return address(this).balance; // returns the balance of this contracts address built-in function
}
}
pragma solidity 0.7.5;
import "./Destroyable.sol";
interface GovernmentInterface{
function addTransaction(address _from, address _to, uint _amount) external payable;
function getBalance() external view returns(uint);
}
contract Bank is Destroyable {
GovernmentInterface governmentInstance = GovernmentInterface(0xa2a7b718Af3CD7F18354Ac5E02235ea6C035BD57);
mapping (address => uint) balance;
event depositDone(uint amount, address depositedTo);
event transferSent(uint amount, address sentFrom, address sentTo);
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 onlyOwner {
//msg.sender is an address
require(balance[msg.sender] >= amount, "Insufficient amount to transfer");
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
}
function getBalance() public view returns (uint) {
return balance[msg.sender];
}
function transfer(address recipient, uint amount) public payable {
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);
governmentInstance.addTransaction{value: 1 ether}(msg.sender, recipient, amount);
emit transferSent(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;
}
function getThisBalance() public view returns(uint) {
return address(this).balance; // returns the balance of this contracts address built-in function
}
}
Hi @Shadowstep2003,
I’ve deployed and tested your contracts and your transfer() function executes successfully, including the external function call and value transfer to Government.
The most common cause of the transfer() function reverting, with the error message you’ve received, is not changing the Government contract’s address, which is assigned to your governmentInstance. Have a look at this post, which explains the issue and how to resolve it. It also explains why the error message you got isn’t very helpful.
A couple of other observations about your Bank contract …
(1) Notice that the address calling the transfer() function is not sending ether from their external address to the Bank contract (as they do when calling the deposit function). The transfer performed is an internal reallocation of funds between two individual users. A function only needs to be marked payable when it needs to receive ether. So, unlike the deposit() function, we don’t want to mark transfer() as payable in its function header.
Within the transfer() function, the external function call to addTransaction() does involve ether being transferred from Bank to the Government contract (as a tax on the transfer transaction, or as a fee for recording it in the Government transaction log). But this value transfer only requires the receiving function (addTransaction in Government) to be marked payable, and not the sending function (transfer in Bank).
Marking the transfer() function as payable doesn’t prevent the code from compiling, or the function from executing, but it is bad practice and poor risk management not to restrict a function to non-payable if it doesn’t need to receive ether, because if someone sends ether by mistake, this amount will still be transferred to the contract. For example, if the caller sent an ether value to the transfer() function by mistake (as well as calling it with a transfer amount and a recipient address) then this ether would be added to the Bank contract balance and potentially lost to the user, because such a deposit wouldn’t also be added to the user’s individual account balance in the mapping (only the transfer amount is deducted from their balance). However, the function is much securer if we leave it as non-payable, because then, if the caller made the same mistake, the function would revert, and the ether would not be sent.
(2) You’ve applied the onlyOwner modifier to the withdraw() function, but this means that only the contract owner can withdraw funds up to their own individual balance. 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? 
Let me know if you have any questions about these points, or if you’re still having problems calling the transfer() function.
What is the base contract?
The base contract is the “parent” contract from which we could derive their properties into child
contracts.
Which functions are available for derived contracts?
We have access to all the functions that are declared public or internal.
What is hierarchical inheritance?
It’s a type of inheritance in which we can have multiple derived contracts from the parent.
Hi @josejalapeno,
Q1 
Q2 Which functions are available for derived contracts?
No … it is the visibility of a function in the base contract, which determines if it is inherited by the derived contract or not.
Functions in the base contract with public or internal visibility are inherited, but those with private or external visibility are not available for derived contracts.
Q3 What is hierarchical inheritance?
I think you’ve understood this. But just to confirm …
Hierarchical inheritance is “where multiple child contracts are derived from a single base contract”.
(Single inheritance is where a single child contract is derived from a single base contract.)
Let me know if you have any questions
thanks for clarifying the number 2 question and yes I understand the number 3 question.
What is the base contract?
the contract has a parent known as the derived class and the parent contract is known as a base contract
Which functions are available for derived contracts?
all public and internal scoped
functions and state variables are available to derived contracts。
a single contract acts as a base contract for multiple derived contracts.
Nice answers @michael356 
This part of your answer is correct …
But this part is actually an error in the article …
The parent contract (or base contract) can also be called the base class. These are all alternative terms for the same thing.
The child contract (or derived contract) can also be called the derived class. These are also three alternative terms for the same thing.
Let me know if you have any questions.
The “base contract” refers to the parent contract in a parent-child relationship.
All public and internal scoped functions and state variables are available to the derived contracts. And
A hierarchical inheritance is where a single contract forms the base for multiple derived contracts.
What is the base contract?
If there is a multiple, hierarchical, level inheritance between two contracts or more, base contract define the parent contract (top-level).
Which functions are available for derived contracts?
Public and internal functions are available for derived contracts.
What is hierarchical inheritance?
This is when we have two or more child contract for a derived class (the base contract).
Nice answers @_Vincent
A couple of clarifications …
Just to be clear, a base contract is any parent contract that is inherited by a derived contract. Within an inheritance structure there can be more than one base contract e.g.
// Multi-level inheritance structure
contract A { ... }
contract B is A { ... }
contract C is B { ... }
In this multi-level inheritance structure:
C is a derived contract
A is a base contract… but not the only base contract …
B is both a derived contract (of A), and a base contract (to C)
Basically…
parent contract = base contract
child contract = derived contract
They are just alternative terms for the same thing.
This is correct apart from the bit I’ve highlighed in bold. There is actually an error in the article where it says that a parent contract can also be called the derived class. The correct terminology is as follows …
The parent contract (or base contract ) can also be called the base class. These are all alternative terms for the same thing.
The child contract (or derived contract ) can also be called the derived class. These are also three alternative terms for the same thing.
So, hierarchical inheritance is where two or more child contracts are derived from the same, single base contract (base class).
Let me know if you have any questions 
Thank you so much for taking time for this detailed answer.
And thank you for the correction on last question ; here is an imoprtant information to highlight in official video, before people read the linked blog.
Hey everyone,
a question regarding the course video :
We need to compile the lowest level in order to compile all contracts (in the course instance, it gives to compile the Bank contract for compiling Bank and Ownable contracts both).
But, in Remix, what if there are 50 derived contracts from Owner base contract. We have to compile the 50 files one by one in Remix ?
Thanks,
Vincent.
Hi @_Vincent,
Don’t confuse compiling with deploying… Each file is compiled individually. However, even if all of the code in a derived contract’s file is correct, if any of the contracts it inherits from contain compiler errors, then the derived contract also won’t compile. This is because when you compile a derived contract, the resulting bytecode includes all of the inherited functionality as well.
Both files are still compiled separately in Remix. Remix can do this automatically for you as you code. If you haven’t already, you should check the Auto compile box under Compiler Configuration in the Solidity Compiler panel.
Once you’ve finished coding both contracts, and checked they have both compiled successfully (although if Bank compiles, then Ownable must have also compiled, for the reason I’ve explained above), you now just need to deploy Bank.
Essentially, yes… but not if you’ve been auto-compiling as you code. In the next course, once you start using Truffle and Ganache, before deployment, you will compile multiple files all at the same time using just one command in the terminal.
If you had 50 derived contracts, all inheriting from a single parent contract (possible, but unlikely), and all in separate files, then you would be compiling 51 .sol files, and then deploying 50 derived contracts.
If you had a single derived contract inheriting from 50 parent contracts (highly unlikely!), then you would still be compiling 51 .sol files, but only deploying 1 derived contract.
bank.sol
pragma solidity 0.7.5;
import "./destroyable.sol";
contract Bank is Destroyable {
mapping(address => uint) balance;
function deposit() public payable returns (uint) {
balance[msg.sender] += msg.value;
return balance[msg.sender];
}
function widthdraw(uint ammount) public returns (uint) {
require(balance[msg.sender] >= ammount);
balance[msg.sender] -= ammount;
msg.sender.transfer(ammount);
return balance[msg.sender];
}
function getBalance() public view returns (uint) {
return balance[msg.sender];
}
function destroy() public {
msg.sender.transfer(address(this).balance);
destroy_contract(msg.sender);
}
}
ownable.sol
pragma solidity 0.7.5;
contract Ownable {
address payable owner;
modifier onlyOnwner {
require(msg.sender == owner);
_;
}
constructor() {
owner = msg.sender;
}
}
destroyable.sol
import "./ownable.sol";
pragma solidity 0.7.5;
contract Destroyable is Ownable {
function destroy_contract(address payable owner) onlyOnwner internal {
selfdestruct(owner);
}
}
Hi @Spiros_Kaftanis,
Your solution works, you have correctly coded the inheritance structure, and you have met all of the assignment objectives
However, you do have some redundant code which can be removed. This will reduce gas costs (although any impact from this would be inconsequential), but more importantly it will reduce the potential risk from bugs and attacks…
One of the key benefits of inheritance is avoiding code duplication. If you change the visibility of the destroy_contract() function in Destroyable to public, it will still be inherited, and it will also be available for the contract owner to call directly, instead of having to call it via the additional destroy() function you’ve added to Bank. The onlyOwner modifier you have applied to destroy_contract() ensures that only the contract owner can call this function and trigger selfdestruct.
You also don’t need to transfer the contract balance to the owner using …
… because the pre-defined functionality of selfdestruct takes care of this automatically.
selfdestruct takes a single payable address argument, so that, as well as destroying the contract and removing it from the blockchain, it can also transfer any remaining contract balance to the address it is called with.
The onlyOwner restriction is not applied to destroy(), so any address can call it. If this happens, the transaction, and any associated changes to the contract state, will still be reverted thanks to the onlyOwner restriction applied to destroy_contract(), but only once the code in the destroy() function body has already been executed. It is safer to prevent as much unnecessary code from executing as possible in the first place.
If destroy() is removed from Bank, destroy_contract() no longer needs the payable address parameter, and so this can also be removed. This also means that there is no longer any confusion over which owner the selfdestruct argument references: the inherited payable address state variable, or the payable address parameter. Due to the onlyOwner modifier, by the time selfdestruct is actually called, both of these “owners” can only be the same address; but by making this code “cleaner”, we also reduce the risk of any bugs creeping in.
Let me know if anything is unclear, or if you have any questions about these points
Edit: I have added the previously missing parantheses to my code and now it works.
Hi @jon_m,
I played around with the topic value call .
It worked fine with the bank contract we built during this course (I did the example that Filip did in the video).
However, when I built and tested the following very simple contract examples, it doesn’t work and I can’t figure out why.
This is the external contract
pragma solidity 0.7.5;
contract valueCallExternal{
function withdrawValue() external payable {
}
function contractBalance() public view returns(uint) {
return(address(this).balance);
}
}
This is NOT the external contract
pragma solidity 0.7.5;
interface thisInterface{
function withdrawValue() external payable;
}
contract valueCall{
thisInterface interfaceInstance = thisInterface(*address of external conctract*);
function deposit() public payable {
}
function transfer() public {
interfaceInstance.withdrawValue{value: 1 ether}();
}
function contractBalance() public view returns(uint) {
return(address(this).balance);
}
}
I can deposit ether, retrieve the contract balance, but when I run the function “transfer”, no value is transferred to the external contract. After the deployment of both contracts, when I hold the mouse cursor over the button for the “transfer” function, it says: transfer - transact (not payable)
I assume this is the source of the error, but I don’t know how to solve it.