contract Bank is Ownable, Destroyable { … }` also works but is unnecessary… is this the version that you say wasn’t explained? …
Yes I was wondering about that, but I realised it might be the Multiple inheritance.
Therefore, since we are using solidity 0.7.5; we can just use msg.sender as a payable address.
That is more cooler, instead of making another variable receiver. Thanks a lot.
Yes, it’s much cleaner. And if we use Solidity v0.8, we can just do the following …
function taminate() public onlyOwner {
selfdestruct(payable(msg.sender));
}
… to convert msg.sender from a non-payable address to a payable one.
Hi @farhan.qureshi,
Your solution works, you’ve correctly coded a multiple inheritance structure, and you have met all of the main assignment objectives 
Some observations and comments …
(1) As well as destroying the contract, selfdestruct will also automatically transfer the remaining contract balance to the address argument it is called with (in our case, the contract owner’s address). This is all part of the pre-defined selfdestruct functionality.
Each user’s individual contract balance represents a share of the total contract balance, and the contract owner’s individual balance is no different. So, there is no need for the contract owner’s individual balance to be withdrawn separately from the contract before selfdestruct is called, because selfdestruct itself will transfer this as part of the total contract balance transferred to the contract owner’s external address.
If you remove the function call withdraw(balance[msg.sender]); from your destroy() function in Bank, you will see that calling this function still produces exactly the same results.
(2) One of the key reasons for using inheritance is to avoid code duplication. If you change the visibility of the destroy() 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. You can then remove the whole of the additional destroy() function from Bank, as long as you also do the following:
selfdestruct so that it still references the contract owner’s address and is also payable. This can be done in 3 different ways. Which would you choose?
Let me know if anything is unclear, or if you have any questions 
Thanks again I got it now.
It is not working correctly. Please help advise me what is wrong. Maybe because of this 0.8.10 version I need to take a different path? I see that that payable needs to be declared for a specific address or let’s say more correctly adresses in code need to be declared as payable clearly.
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.10;
import "./Ownable.sol";
import "./Destroyable.sol";
contract EtherBank 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) onlyOwner public returns (uint){
require(balance[msg.sender] >= amount);
balance[msg.sender] -= amount;
payable(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);
require(msg.sender != recipient);
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;
}
}
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.10;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner {
address payable receiver = payable(msg.sender);
selfdestruct(receiver);
}
}
/ SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.10;
contract Ownable {
address public owner;
modifier onlyOwner {
require (msg.sender == owner);
_; //
}
constructor () {
owner = msg.sender;
}
}
Hi @Jaka,
Your solution works and meets all of the assignment objectives:
selfdestruct if they need to destroy the Bank contract.selfdestruct is triggered, any remaining contract balance is all automatically transferred to the contract owner’s external address. This is because, via the local payable address variable receiver, msg.sender is the payable address argument which selfdestruct is called with; and msg.sender can only be the owner address in this function because, due to the onlyOwner modifier, the destroy() function can only be called successfully by this address.Your syntax is correct for Solidity v0.8.
Only for addresses that need to be payable, and are not already payable by default.
In Solidity v.0.7 msg.sender is a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address. However, from v.0.8 msg.sender is non-payable by default. The transfer method must be called on a payable address, and selfdestruct() must be called with a payable address argument, and this is why you needed to convert msg.sender using payable(msg.sender) in the following two instances:
(1) In the withdraw() function in Bank
(2) In the destroy() function in Destroyable
In fact, you don’t need this additional step with the local variable receiver. Instead, you can just convert msg.sender directly within the selfdestruct() function call. In my opinion, this results in more concise and cleaner code …
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.
Without the payable keyword, an address variable will always be non-payable by default. But notice that, in your solution, the owner address used within the onlyOwner modifier (in Ownable) doesn’t need to be explicity converted to a payable address. This is because the condition within this modifier doesn’t need it to be a payable address.
Another example of where a payable address isn’t required in your code is the assignment of msg.sender to owner in the constructor (in Ownable).
A couple of additional observations …
We can also make the inheritance structure 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’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 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? 
Let me know if anything is unclear, if you have any further questions, or if you are still having trouble getting your code to work.
Thanks Jon! Will go back through the course again and do so!
Order of inheritance: Ownable -> Destroyable -> EthBank. No changes were made to EthBank.
Code:
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //runs the functions
}
constructor(){
owner = msg.sender;
}
}
//new file
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function close() public onlyOwner {
selfdestruct(owner);
}
}
//new file
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract EthBank is Destroyable {
//code hereafter was unchanged
Let me know if any improvements can be made. Thank you!
An excellent solution @nbkeaton 
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.
// The Destroyable contract inherits from Ownable
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function selfDestruct() public onlyOwner {
selfdestruct(msg.sender);
}
}
// the contract to be deployed (BankDestroyed) inherits both from Ownable and Destroyable
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract BankDestroyed is Ownable, Destroyable {
Ownable.sol
pragma solidity 0.7.5;
contract Ownable{
address public owner;
modifier onlyOwner{
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
}
Destroyable.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroyContract() public onlyOwner{
selfdestruct(owner);
}
}
Bank.sol
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract Bank is Destroyable {
File Name: 15-1_Ownable.sol
pragma solidity 0.7.5;
contract Ownable{
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner, "Address does not have admin access");
_; // This underscore basically means "continue with running function"
}
constructor(){
owner = msg.sender;
}
}
File Name: 15-3_Destroyable.sol
pragma solidity 0.7.5;
import "./15-1_Ownable.sol";
contract Destroyable is Ownable {
function _destroy() public onlyOwner {
msg.sender.transfer(address(this).balance);
selfdestruct(owner);
}
}
File Name: 15-2_BankContr.sol (Only first few lines of code)
pragma solidity 0.7.5;
import "./15-3_Destroyable.sol";
contract Bank is Destroyable {
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));
}
}