Hey Alex,
You’re missing the parentheses at the end of the external function call…
Remember you always need the parentheses even if there aren’t any arguments. What’s probably thrown you is the fact that you have the curly brackets, and the ether value you are sending is effectively an argument… You need the following …
interfaceInstance.withdrawValue{value: 1 ether}(); 
This isn’t the problem. Your transfer() function isn’t payable, so that’s correct. The caller isn’t sending ether to the valueCall contract.
It’s the external function withdrawValue that is payable, and it’s the valueCall contract which is calling that and sending ether to the valueCallExternal contract.
Thank you and your eye for detail 
I missed the parentheses - that was it!
Just looked over my code again and realized my misunderstanding of the issue at hand. I think I was so consufed by the error resulting from the missing parentheses that I was trying to find a possible explanation for this error…
Thank you once again for your very helpful reply!
You’re very welcome @Alex_13,
Great to see you are doing your own experimentation with the code you’ve been learning. That’s a great way to consolidate what you’ve learnt 
Yes, I’ve been there myself many a time… 
Thank you! That’s clear!
This is the Ownable.sol file
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner{
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
}
This is the Destructable.sol file
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable{
function close() public {
selfdestruct(owner);
}
}
This is the Bank.sol
pragma solidity 0.7.5;
import "./Destructable.sol";
contract Bank is Destroyable{
mapping(address => uint) balance;
event balanceAdded(uint amount, address depositedTo);
event transferred(uint amount, address indexed transferredTo, address TransferredFrom);
function Destructthis () public onlyOwner {
Destroyable.close();
}
function deposit() public payable returns (uint){
balance[msg.sender] += msg.value;
emit balanceAdded(msg.value, msg.sender);
return balance[msg.sender];
}
function getBalance() public view returns(uint){
return balance[msg.sender];
}
function withdraw(uint amount) public onlyOwner returns(uint){
require(balance[msg.sender] >= amount);
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
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 transferred(amount, recipient, msg.sender);
assert(balance[msg.sender] == previousSenderBalance - amount);
}
function _transfer(address from, address to, uint amount) private{
balance[from] -= amount;
balance[to] += amount;
}
}
Hi @Sil,
Your solution is almost there… Everything compiles, you’ve correctly coded a multi-level inheritance structure, the contract can be destroyed, and on destruction any ether still remaining in the contract will be transferred to the contract owner’s external address 
You just have one problem to resolve…
If you deploy Bank in Remix, you will notice that both the Destructthis() function and the close() function are available to call externally. This is because the inherited close() function has public visibility, allowing it to be called from both within the Bank contract and externally. So, while you have appropriately restricted access to selfdestruct to the contract owner (using onlyOwner) if Destructthis() is called, this doesn’t stop any address from avoiding this constraint and triggering selfdestruct by calling close() directly, because onlyOwner isn’t applied to the close() function. The funds are safe, because only the contract owner can receive them, but the fact that any address can actually trigger selfdestruct in the first place is obviously not ideal 
As the close() function is already available to be called externally, there is no need to include Destructthis() in the Bank contract at all. You just need to make sure that the onlyOwner modifier is added to the close() function header, so that only the contract owner can call it and destroy the Bank contract.
Some additional comments …
(1) If you did need to call close() from the Bank contract, you could do it using just …
close();
You wouldn’t need to call it using …
Destroyable.close();
… because there is only one implementation of close(). We would only need to include the contract name, or use super.close() , if there was another implementation of close() in Bank, overriding close() in the parent contract Destroyable, and we needed to specify calling the overridden implementation in the parent as opposed to the overriding implementation in the derived contract. The following link is to the relevant section on this in the Solidity documentation …
https://docs.soliditylang.org/en/latest/contracts.html#inheritance
(2) You’ve modified the withdraw() function with the onlyOwner modifier, but this means that only the contract owner will be able to withdraw funds, whereas 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? I think during the course we were adding and removing onlyOwner from various functions to demonstrate different scenarios. Somehow it ended up being left in the withdraw() function.
(3) 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. However, in general, an emit statement is probably better placed after an assert statement.
Let me know if anything is unclear, or if you have any questions 
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?
Public and internal functions.
What is hierarchical inheritance?
Hierarchical inheritance is where a single contract acts as a base contract for multiple derived contracts.
Hi @Joeyj007,
Q2 & Q3
This part of your answer to Q1 is correct.
But the first part of your answer …
There is actually an error in the article when it refers to class, here.
Contracts in Solidity operate in a similar way to classes in other object-oriented programming languages, and therefore the following are alternative names for the same thing:
parent contract = base contract = base class
child contract = derived contract = derived class
Let me know if you have any questions
Hi, I have a couple of questions on external functions and value calls to them.
About the addTransaction function in the Government contract for logging transactions in an array… I thought since it is an external function it can only be called by external contracts. I can execute it using the Remix interface by manually inputting addresses and an amount and it then adds it as a transaction to the array. Is this normal? I thought it should only be executable by external contracts.
About value calls to external functions… I’ve done as Filip did in the video and added {value: 1 ether} to the function call, added the payable keyword in the relevant places and added a getContractBalance function to the Government contract to check it’s working and indeed it all seems to work. My question is, where does this 1 ether come from? I make transactions in the Bank contract and the funds transfer just fine, nothing gets deducted from the transfer, but because the transaction is being logged by the external Government contract and I’ve added 1 ether of value to that function call, the Government contract’s balance is increasing by 1 ether with each transaction it logs. But where is the ether coming from? It seems to just get created out of nothing and I feel like I’m missing something.
Any help with either of the above would be greatly appreciated!
Hey @tom88norman,
Good to see you back here in the forum
Yes this is correct.
… by external contracts AND from any external “service” outside of the EVM/blockchain e.g. from Remix, or from the front-end interface of a dapp …
So, to put it another way, from Remix you can call both the public and external functions of a deployed contract.
Other than from an external “service”, external functions can only be called from external contracts. But as you know, public functions can also be called from anywhere else: from within the same contract, from within any derived contracts, and also from external contracts.
Let me know if anything is still unclear or if you have any further questions about this. I’m now going to have a look at your other question
Hey again, Tom,
Yes, unfortunately it’s not magic You’ve got two things going on here…
(1)
The external address — the one showing in the Account field near the top of the Deploy & Run Transaction panel in Remix — which is calling the transfer() function, is the msg.sender of the internal transfer of funds within the Bank contract. In terms of the funds transferred between msg.sender and recipient, no actual ether enters or leaves the Bank contract. That’s why you won’t see any change to either of their external address balances in the Account field. What happens is effectively a reallocation of the contract’s total ether balance i.e. the msg.sender's share of the total contract balance (or their entitlement) decreases, and the recipient’s share (or entitlement) increases by the same amount. This is why you will only see a change in their individual user balances in the mapping, which you can check by calling getBalance() with each address.
(2)
The external function call within the transfer() function…
governmentInstance.addTransaction{value: 1 ether}(msg.sender, recipient, amount);
… is not invoked by the initial caller of the actual transfer() function (i.e. the user with an individual balance in the Bank’s mapping who is transferring funds to another user). Instead, the external function addTransaction is called by the Bank contract address itself (i.e. the address it’s deployed at). So the value of 1 ether is transferred from the Bank contract ether balance to the Government contract ether balance. You will only see the origin of the 1 ether if you add a separate getContractBalance() function to the Bank contract, so you have one in both contracts…
function getContractBalance() external view returns(uint) {
return address(this).balance;
}
Check both contract balances both before and after calling the transfer() function, to see how they change. The Bank contract balance should decrease by 1 ether, and the Government contract balance should increase by 1 ether.
Note that, even though addTransaction() is called by the Bank contract address, the msg.sender it is called with (as the 1st argument) references the address which initially called the transfer() function (and not the Bank contract address).
Without the external function call, the Bank contract balance should always be equal to the sum of all of the separate, individual user balances in the mapping; but with the external function call, the Bank contract balance will be less than the sum of these individual balances by an amount equal to the total fees — or tax, or levy, depending on how you wish to interpret the ether value transfer — “paid” by the Bank contract to the Government contract per transfer transaction.
Our particuar example code is based on a contractual relationship between a bank and the government. The idea is that the government needs to collect some kind of tax on each banking transaction that is an internal bank transfer. To keep things simple in our example, the government is collecting a fixed amount of tax on each transfer. I would suggest an amount much smaller than 1 ether would be more appropriate, for example any value from 1 wei up to 1 gwei. This amount (which is the amount of tax paid by the Bank, and not the amount transferred from one bank account holder to another) will be automatically transferred from the Bank contract balance to the Government contract balance on each successfully executed transfer transaction. For record keeping purposes, the relevant data for each transfer is also sent from Bank to Government and stored in the government’s transaction log (an array).
Instead of as a tax, another way to interpret this transfer of value between the two contracts could be as payment, or fees, for some kind of record keeping service i.e. the Bank contract pays the Government contract for storing all of its transaction data.
However we interpret this payment from one contract to the other, it demonstrates (albeit in an overly simplistic way) how we can program automatic interactions between individual contracts for the purposes of both data and value transfer. Hopefully, this example shows the potential for these sorts of programmed interactions to be applied to many different use cases.
Anyway, I hope that helps to clarify things. Do let me know if anything is still unclear, or if you have any further questions
Hi Jon,
Thanks so much, as always, for taking the time to give such a detailed answer.
Yeah I do (finally) get the difference between the balances of the individual users tracked with a mapping vs the overall contract’s balance. It just never occurred to me that it was possible for there to be a discrepency between them (as in, the sum of all mapped balances vs the contract’s actual balance). But I guess it makes sense if you think about it in the context of an actual bank. And given the fact that the balance mapping is just a number that isn’t tied to any actual funds.
Anyway, your explanation really helped as always, thanks again.
I’ve been neck-deep in the multisig wallet project for the last few days so I’m about to post my first janky attempt at that wish me luck!
Hey Tom,
Good luck!
From what you’ve said in your post it sounds like lots of things have now clicked into place 
I understand what you mean here. I like to think of the mapping balances as accounting entries, and the additions and deductions we make to these balances in the various functions as accounting adjustments. These balances are “tied” to funds, because they reflect each individual user’s share of the total contract balance, effectively their entitlement to transfer ether out of the contract to an external address. If it wasn’t for these individual balances, we would just have a pool of funds in the contract without any way to record or control the allocation of ownership of these funds to individual external addresses, and how this changes over time.
Ownable Contract
pragma solidity 0.8.10;
contract Ownable {
address public owner;
modifier onlyOwner {
require (owner == msg.sender);
_;
}
constructor(){
owner = msg.sender;
}
}
Destroyable Contract
import “.\Ownable.sol”;
pragma solidity 0.8.10;
contract Destroyable is Ownable {
function terminate() public OnlyOwner {
address payable receiver = msg.sender;
selfdestruct(receiver);
}
}
Bank Contract
import “.\Destroyable.sol”;
pragma solidity 0.8.10;
contract Bank is Destroyable {
mapping(address => uint) balance;
event depositDone(uint amount, address indexed depositedTo);
function DestroyThis() public onlyOwner {
Destroyable.close();
}
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 @PabloAAAJ,
Your attempt to implement a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.
Unfortunately, your Destroyable and Bank contracts don’t compile for various reasons, and so your Bank contract can’t be deployed…
(1) For the current version of your code, you should be getting compiler errors for both of your import statements…
This is because each file path should have a forward slash instead of a back slash.
(2) Once you’ve corrected the import statements, you should now be getting a compiler error for this line …
The error here is an upper/lower case inconsistency with one of the letters in your modifier name. Let the compiler, and the error messages it generates, help you find these types of spelling-related errors.
(3) The next compiler errors that are generated are for the following two lines of code:
In the terminate() funtion in Destroyable…
In the withdraw() function in Bank…
Prior to Solidity v0.8 msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address (such as in the above two lines of code. However, you are using Solidity v0.8.10, and from v.0.8 msg.sender is non-payable by default, and so this means that in the above two lines of code you need to explicity convert msg.sender to a payable address using the syntax payable() …
// In the terminate() function in Destroyable
// selfdestruct() must be called with a payable address argument
address payable receiver = payable(msg.sender);
selfdestruct(receiver);
// In the withdraw() function in Bank
// the transfer method must be called on a payable address
payable(msg.sender).transfer(amount);
In the terminate() function, a concise alternative to using the additional receiver variable is to call selfdestruct() with msg.sender directly, and convert it to a payable address within the function call itself…
selfdestruct(payable(msg.sender));
Have a look at this post for further details about the use of the additional local variable in the model solution.
(4) The next compiler error is generated for the function call within the DestroyThis() function in Bank…
Again, the error message will make it clear what the problem is. You are trying to call a function in the Destroyable contract that doesn’t exist!
The inherited terminate() function has public visibility and so it is available to call within Bank. This means that you can just call it as follows …
function DestroyThis() public onlyOwner {
terminate();
}
However, one of the key reasons for using inheritance is to avoid code duplication. The terminate() function has public visibility and so, when Bank is deployed, terminate() is also available to be called directly by the contract owner’s external address whenever they want to destroy the contract, instead of having to call it via the additional DestroyThis() function you’ve added to Bank. Therefore, you can remove the whole of the DestroyThis() function from Bank.
By the way I think you missed out the Events Assignment, because you are missing a Transfer event in your Bank contract, and a corresponding emit statement in its transfer() function.
Let me know if anything is unclear, if you have any questions, or if you need any help correcting your contracts, deploying Bank, and getting it to do what it should
Thanks a million for your feedback!
I’ve corrected the typos and I’m working on those events!
Hey guys hoping someone can help me with a couple questions i have regarding the Inheritance Assignment with the 3 contracts… Bank.sol, Destroyable.sol. and Ownable. sol. With bank.sol I noticed I could not withdraw or transfer money in Remix without adding “payable” to those functions. Is it correct that i would have to add “payable” in the header of the withdraw function and transfer functions in order to withdraw and transfer money?
One other question as welll… I noticed the “withdraw” function in bank.sol does not adjust the balance after the withdrawal. Shouldn’t there be a line to do this so we can accurately reflect the balance, like balance[msg.sender] -= amount. Something like this?