Nice solution @DNC 
We can also streamline the inheritance structure by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
Let me know if you have any questions 
Hi @jrobbins,
Your implementation of 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.
However, your Bank contract doesn’t compile and so can’t be deployed. This is because of the compiling error you should be getting for the following line of code in your Destroyable contract…
If you read the compiler error you will see that owner needs to be explicity converted to a payable address. This is because owner is defined in your Ownable contract as a non-payable address, and the method selfdestruct requires a payable address argument.
You can easily fix this is several different ways. Which would you choose? If you’re not sure, have a look at some of the other students’ solutions, and the feedback and comments they’ve received, posted here in this discussion topic. You will find a lot of useful information here.
This is the only error; the rest of your code is correct.
Hi @281Cypher,
This is a very well-presented solution which works and meets all of the main assignment objectives
Your implementation of 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.
When your contract is destroyed, the remaining contract balance is successfully transferred to the contract owner’s external address. But you don’t need to include the additional line of code in your _destroy() function to achieve this…
This line of code achieves the desired result and from a technical point of view it is well coded. However, as well as destroying the contract, selfdestruct will also automatically transfer the remaining contract balance to the payable address argument it is called with (in our case, the contract owner’s address). This is all part of the pre-defined selfdestruct functionality. So, if you remove this additional line of code from your _destroy() function, you will see that calling this function still produces exactly the same results.
Let me know if anything is unclear, or if you have any questions
Hi!
Here are my contracts!
Ownable.sol:
// SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
contract Ownable {
address payable _owner;
modifier onlyOwner() {
require(_owner == msg.sender, "Caller is not the owner");
_;
}
}
Destroyable.sol:
// SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable{
event selfdestructed(bool destructed, address indexed destroyedBy);
function close() public onlyOwner {
selfdestruct(_owner);
emit selfdestructed(true, msg.sender);
}
}
and Bank.sol (excluding functions):
// SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract Bank is Destroyable{
mapping(address => uint) balance;
event depositDone(uint amount, address indexed depositedBy);
event withdrawDone(uint amount, address indexed withdrawTo);
event sendDone(uint amount, address indexed sentTo);
constructor() {
_owner = msg.sender;
}
Hi @Davelopa,
This is a very well-presented solution which works and meets all of the main assignment objectives
Your implementation of 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.
A few comments …
(1) As the constructor initialises the _owner state variable in Ownable, it is better to place it in Ownable as well.
By placing all of the code and functionality related to contract ownership within a single, separate contract (Ownable), you will improve the modularity of your code (one important reason for using inheritance) and will keep your code more organised.
It will also reduce any potential code duplication in the event that Ownable is re-used and inherited by other derived contracts, because the constructor won’t then have to be repeated in each derived contract for Ownable to work as it should. Reducing code duplication and increasing code re-usability are other important reasons for using inheritance.
(2) Your selfdestructed event is certainly a good idea but, after calling close() and destroying the contract, if you check the transaction receipt you will notice that the event data you intended to emit hasn’t been logged. You are right that an emit statement should be placed after the code associated with the event itself, in order to minimise the risk of the event being emitted erroneously. However, in this case, if your emit statement is placed after the call to selfdestruct(), it will never be reached, because the contract will have been destroyed, and its code removed from the blockchain, once selfdestruct() has executed.
You could fix this by placing the emit statement before the selfdestruct() function call, but I don’t think this is wise, because we would be emitting the event before the actual event itself (contract destruction) has occurred. On the other hand, if selfdestruct() failed, then the function would revert, including the emitted event, but I would still be reluctant to place the emit statement first, for the reasons I’ve mentioned above in terms of reducing risk.
The reason why I say that emitting an event is a good idea (at least in theory) is because of a serious drawback of using the selfdestruct() function, the potential repercussions of which need to be fully understood before choosing to implement it. As you may, or may not, already have realised, after selfdestruct() has been executed, successful calls can still be made to the destroyed contract’s functions! As a result of this, if a smart contract deployed on the mainnet is ever destroyed, all users need to be notified and informed that they must not send any more funds to that contract address. If a user doesn’t know that the contract has been destroyed, and goes ahead and sends funds to it (e.g. by calling the deposit function), then those funds will be lost. You can see this for yourself by calling deposit() with an ether value. Remix will notify you that the transaction has been successful, and you will see the ether amount has been deducted from the external address balance. But if you add a function that gets the ether contract balance and call that (not the getBalance function), you will see that the contract has a zero balance!
It’s worth bearing in mind that emitting an event doesn’t necessarily mean that users will see that infomation, and so doesn’t actually prevent them from subsequently calling the destroyed contract’s functions, incurring a gas cost, and losing any ether sent. There would still need to be some form of communication to all users informing them of the contract’s destruction, and what the consequences and implications of this are in terms of what they must and mustn’t do from now on.
This is obviously hardly an ideal situation! In practice, other types of security mechanisms, such as pausable or proxy contracts, are implemented to deal with these kinds of emergency situations. But this is out of the scope of this introductory course. You will learn about more advanced security techniques and practices if you go on to take the Ethereum Smart Contract Security Course.
Let me know if you have any questions
By the way, I don’t think I’ve seen these events in any of the previous code you’ve submitted. It would be good to see how you’ve implemented them i.e. their corresponding emit statements in the relevant functions. Is one of these events for the transfer() function? Implementing and emitting an event for the transfer() function was the task for the Events Assignment, but it doesn’t look like you posted a solution for that one.
Ownable.sol:
contract Ownable {
address public owner;
constructor() {
owner = msg.sender;
}
modifier onlyOwner {
require(msg.sender == owner);
_;
}
}
Destroyable.sol:
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner {
address payable receiver = msg.sender;
selfdestruct(receiver);
}
}
Bank.sol:
pragma solidity 0.7.5;
import "./Destroyable.sol";
import "./Ownable.sol";
contract Bank is Ownable, Destroyable {
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 onlyOwner 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;
}
}
For the Parent Bank.sol contract added:
import “./Ownable.sol”;
import “./Destroyable.sol”;
contract Bank is Ownable, Destroyable{…}
Ownable Contract:
pragma solidity 0.8.7;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_; // run the function
}
constructor(){
owner = msg.sender;
}
}
Destroyable Contract:
pragma solidity 0.8.7;
import “./Ownable.sol”;
contract Destroyable is Ownable{
function close() public onlyOwner {
selfdestruct(payable(owner));
}
}
Hi @0xBrito,
Your code meets the assignment objectives. Your inheritance structure is well coded, but we can make it more streamlined by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
Some additional comments …
(1) In your destroy() function, a concise alternative to using the additional receiver variable is to just call selfdestruct() with msg.sender directly…
selfdestruct(msg.sender);
You are using Solidity v0.7.5, and prior to 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 e.g. as the payable address argument in the selfdestruct() function call, or as the payable address the transfer method is called on. However, from Solidity v.0.8 msg.sender is non-payable by default, which would mean having to explicity convert msg.sender to a payable address when necessary e.g.
selfdestruct(payable(msg.sender));
Have a look at this post for further details about the use of the additional local variable receiver in the model solution.
(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 while the Bank contract is deployed and operating normally. The contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so, while the contract is still deployed, it only seems fair that all users should be able to withdraw their funds as well, don’t you think? 
Your withdraw() function header didn’t include the onlyOwner modifier before. I think during the course we were adding and removing onlyOwner from various functions to demonstrate different scenarios, and I think it might have ended up being left in the withdraw() function in the solution code for this assignment by mistake!
Don’t forget to post your solution to the Events Assignment here, and your answers to the Inheritance Reading Assignment here. The Events Assignment is from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course. I think you may have missed it out because you are missing a Transfer event in your code, and a corresponding emit statement in the transfer() function. The Inheritance Reading Assignment is at the beginning of the Inheritance & External Contracts section of the course, and in the article you will learn, amongst other things, about the multi-level inheritance structure I’ve mentioned above, as well as other types of inheritance structures.
Let me know if you have any questions.
Hi @jsilver,
Your inheritance structure is well-coded, and your solution meets the assignment objectives
We can also streamline the inheritance structure by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
It would be good to also see your full Bank contract, because you’re using a different version of Solidity (v0.8) to the one used in the course (v0.7). This is perfectly fine, and is to be encouraged, but as this is the first code you’ve posted for this course, I think it would be helpful to check that you’ve made the necessary change to the syntax for v0.8. There is, actually, only one change that affects this particular contract, but as you haven’t posted your solutions to the earlier Events Assignment and Transfer Assignment, I can then check your Bank contract contains the correct code for these as well.
If you would like to, you can also post your solutions to these assignmnets separately, in the following discussion topics:
Events Assignment https://studygroup.moralis.io/t/events-assignment/28040?u=jon_m
This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course.
Transfer Assignment https://studygroup.moralis.io/t/transfer-assignment/27368?u=jon_m
This is the assignment from the Payable Functions section of the course, where you have the task of completing the withdraw() function.
Data Location Assignment https://studygroup.moralis.io/t/data-location-assignment/27990?u=jon_m
This assignment doesn’t involve the Bank contract. It’s the first assignment of the course, just before the Events Assignment, near the beginning of the Additional Solidity Concepts section, and you haven’t posted your solution for this one either.
Also, please format your code before posting it. This will make it clearer and easier to read. It will also make it easier to copy and paste if we need to deploy and test it.
Follow the instructions in this FAQ: How to post code in the forum
Let me know if you have any questions
Destroyable.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable{
function Destruction() public onlyOwner{
address payable reciever = msg.sender;
selfdestruct(reciever);
}
}
Bank.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable{
...
Ownable.sol
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner);
}
constructor(){
owner = msg.sender;
}
}
Hi @VBence99,
Your solution is almost there… but, as a result of a compiler error in Ownable, your Bank contract cannot be deployed. Your inheritance structure is well coded, and once you correct the error, everything should work as it should.
Your modifier is missing _;
This will have generated a compiler error with an error message indicating exactly what the problem is.
A couple of other comments …
(1) We can make the inheritance struture more streamlined by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
(2) In your Destruction() function, a concise alternative to using the additional receiver variable is to just call selfdestruct() with msg.sender directly…
selfdestruct(msg.sender);
You are using Solidity v0.7.5, and prior to 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 e.g. as the payable address argument in the selfdestruct() function call, or as the payable address the transfer modifier is called on. However, from Solidity v.0.8 msg.sender is non-payable by default, which would mean having to explicity convert msg.sender to a payable address when necessary e.g.
selfdestruct(payable(msg.sender));
Have a look at this post for further details about the use of the additional local variable receiver in the model solution.
(3) You have defined the owner address state variable in Ownable as payable. But this is unnecessary, because you don’t use owner anywhere in your code where it needs to reference a payable address. It would need to be marked payable, however, if you used owner instead of msg.sender (or instead of receiver) to call selfdestruct() with the contract owner’s address, because selfdestruct() takes a single payable address argument…
selfdestruct(owner);
However, because you would only be using owner to reference a payable address once, you could just explicity convert owner to a payable address directly within the selfdestruct() function call, leaving the owner state variable itself defined as a non-payable address…
selfdestruct(payable(owner));
Let me know if anything is unclear, or if you have any questions
Hi, this is my solution:
These three contract files are in the same directory.
ownable.sol
selfdestruct function will send all of the fund of the contract to the owner address so, to receive the fund, the address has to be payable.
// SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
contract Ownable {
address payable owner;
modifier onlyOnwer {
require(msg.sender == owner);
_;
}
constructor() {
owner = msg.sender;
}
}
destroyable.sol
I find that private and internal functions can’t be payable (but I don’t know why) so I set the close() function with the visibility public.
// SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
import "./ownable.sol";
contract Destroyable is Ownable {
function close() public onlyOnwer {
selfdestruct(owner);
}
}
bank.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
import "./destroyable.sol";
contract Bank is Destroyable {
// state variables and functions code of the Bank contract goes here
}
Hi @codingluan,
This is a very well-presented solution which meets all of the assignment objectives 
Your implementation of 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.
It’s because functions should only be marked payable when they need to receive ether and add it to the contract balance. This will only happen when ether is sent to the contract address from an external address e.g. from Remix, a Web3 client (such as the front-end interface of a dapp), or from an external (i.e. non-derived) contract.
private functions can only be called from within the same contract. A contract is deployed at a single Ethereum address and so only has one contract ether balance. The net effect of transferring ether from a contract balance to the same contract balance would be zero, and therefore pointless.
A parent contract’s internal functions can only be called from within the same parent contract or from within its derived contract(s). A derived contract’s compiled bytecode includes the code it inherits from its parent contract(s), which includes any internal functions. This combined bytecode is then deployed as a single contract, at a single Ethereum address, with a single ether balance. Therefore, any ether received by an internal function, could only have been sent from the same contract address balance as the one now receiving it; and so, as with private functions, this would be pointless, because the net effect would also be zero.
This is correct… and the close() function shouldn’t be marked payable anyway, because it doesn’t need to receive ether. It only needs to send any ether remaining in the contract when it’s destroyed to the contract owner’s external address.
The close() function should have either public or external visibility, so that it can be inherited and called externally by the contract owner’s external address if and when they want to destroy the contract. If it is marked private, then it won’t be inherited. If it is marked internal, then it will be inherited, but only available to call from within the deployed contract. This would require adding an additional public or external function which could then be called externally by the contract owner, and which would contain a function call to close(). But this would defeat one of the main reasons for using inheritance in the first place, namely, to re-use code and avoid code duplication.
Just let me know if anything is still unclear, or if you have any questions
Thank you for your informative and clear explanation which helps all these things make sense to me now!
pragma solidity 0.8.11;
contract Ownable {
address internal owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //run the function
}
constructor(){
owner = msg.sender;
}
}
import “./Ownable.sol”;
pragma solidity 0.8.11;
contract Destroyable is Ownable {
function destroy() public onlyOwner {
address payable receiver = msg.sender;
selfdestruct(receiver);
}
And for the header of Bank contract
pragma solidity 0.8.11;
import “./Ownable.sol”;
import “./Destroyable.sol”;
contract Bank is Ownable, Destroyable{}
Hi @frandolz
Your solution is almost there… but, you should find that your Bank contract cannot be deployed as a result of a compiler error in Destroyable. Once you correct this error (see below) everything should work as it should.
Your inheritance structure is well coded, but we can make it more streamlined by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
You have chosen to use the more up-to-date Solidity v.0.8, which is only to be encouraged However, you need to pay close attention to any errors that are generated by the compiler where you wouldn’t expect this based on the syntax taught in this course (Solidity v0.7). Although, in actual fact, the only difference in syntax between these two versions which affects the code covered in this course concerns msg.sender.
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. That’s why the model solution for this assignment (based on Solidity v0.7 syntax) allows msg.sender to be assigned to a payable address variable (receiver) without having to explicitly convert it to a payable address …
However, from Solidity v0.8, msg.sender is non-payable by default, which means having to explicity convert it to a payable address when necessary. And this is why you need to make the following modification to your code in order for it to compile …
address payable receiver = payable(msg.sender);
selfdestruct(receiver);
Once you’ve corrected this, it would be better to also see your complete Bank contract to check that everything there is also coded correctly, because using Solidity v0.8 requires the same modification to be made in one specific place in this contract as well. The compiler should indicate where this is with a red error marker, and the error message should confirm that it is the same type of error as the one in your Destroyable contract.
Some additional comments …
In your destroy() function, a concise alternative to using the additional receiver variable is to just call selfdestruct() with msg.sender directly…
selfdestruct(payable(msg.sender));
Have a look at this post for further details about the use of the additional local variable receiver in the model solution.
If you’d like to fully complete this course, don’t forget to also post your solution to the Events Assignment here, and your answers to the Inheritance Reading Assignment here. The Events Assignment is from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course. The Inheritance Reading Assignment is at the beginning of the Inheritance & External Contracts section of the course, and in the article you will learn, amongst other things, about the multi-level inheritance structure I’ve mentioned above, as well as other types of inheritance structures.
Just let me know if you have any questions
Bank file
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destruct.sol";
contract BankMap is Ownable,SelfDestruct {
. . .
}
Ownable file
pragma solidity 0.7.5;
contract Ownable {
address public owner;
//modifier use as small function that will run first before main function
//use case of modifier is when many functions need to use the same logic
modifier onlyOwner {
//check the person who can add balance is only owner do
require(msg.sender == owner, "only the owner can add money/withdraw");
_; //run the function
}
constructor() {
//msg.sender is the address who deploy contract
owner = msg.sender;
}
}
Destroy file
pragma solidity 0.7.5;
import "./ownable.sol";
contract SelfDestruct is Ownable {
function destroy() public onlyOwner{
selfdestruct (owner);
}
}
Hi @thanasornsawan,
Your solution is nearly there… but your Bank contract cannot be deployed as a result of a couple of compiler errors: one in SelfDestruct, and another in either BankMap or SelfDestruct. Once you correct these errors (see below) everything should work as it should.
Your inheritance structure is mostly well-coded, but we can make it more streamlined by having BankMap explicitly inherit SelfDestruct only, because it will inherit Ownable implicitly via SelfDestruct. This will give you a multi-level inheritance structure.
Compiler errors
(1) BankMap / SelfDestruct
Your Destruct.sol file imports a file called ownable.sol, but your Bank.sol file imports a file called Ownable.sol. You need to make sure that the name of the actual file containing your Ownable contract is spelt correctly and consistently (in terms of whether it starts with a capital or lower case letter) in both import statements. However, if, as I’ve suggested above, BankMap only explicitly inherits SelfDestruct, you can just remove import "./Ownable.sol"; from Bank.sol completely.
(2) destroy() function in SelfDestruct
If you read the compiler error you should be getting for this line of code, you will see that owner needs to be explicity converted to a payable address. This is because owner is defined in your Ownable contract as a non-payable address …
… but the method selfdestruct requires a payable address argument, so that when it’s called, it will automatically transfer the ether balance remaining in the contract to the address it is called with (in your case, owner).
You can easily fix this is several different ways. Which would you choose? If you’re not sure, have a look at some of the other students’ solutions, and the feedback and comments they’ve received, posted here in this discussion topic. You will find a lot of useful information here.
I would also change your error message in the onlyOwner modifier’s require() statement …
You should no longer be restricting access to any of the BankMap contract’s functions (for the reasons I explained in my feedback for your Transfer Assignment) and so this error message needs to either refer to the restriction this modifier applies to the destroy() function in SelfDestruct, or it needs to be more generic so that it makes sense in all of its possible use cases.
Let me know if you have any questions about any of these points
Ownable:
pragma solidity 0.7.5;
contract Ownable {
address owner;
modifier OnlyOwner {
require(msg.sender == owner, "Only the owner can call this function");
_;
}
constructor(){
owner = msg.sender;
}
}
Destroyable:
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable{
function destroy() internal OnlyOwner{
selfdestruct(payable(owner));
}
}
Bank:
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable {