Hi,
I have seen the solution now and just got a question. I wrote below. Is that OK or what fails?
contract Destroy is Ownable {
address payable owner;
function close() public {
selfdestruct(owner);
}
}
In the Ownable we have that msg.sender is = owner
constructor() {
owner= msg.sender; // When contract deployed, the address deploy the contract will be owner
}
Nice solution @mike_sutanto 
In Solidity v0.7, msg.sender is already a payable address by default, and so you can also just pass it directly to selfdestructâŚ
function destroy() public onlyOwner {
selfdestruct(msg.sender);
}
However, from v0.8, msg.sender is a non-payable address by default, and so we would need to code the function body asâŚ
// either
address payable receiver = payable(msg.sender);
selfdestruct(receiver);
// or
selfdestruct(payable(msg.sender));
We can also streamline the inheritance structure by having Bank (your HelloWorld) 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 
//Destroyable.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner {
selfdestruct(owner);
}
}
//Ownable.sol
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //run the function
}
constructor() {
owner = msg.sender;
}
}
//Bank.sol
import "./Destroyable.sol";
contract Bank is Destroyable {
//...
Ownable.sol
pragma solidity 0.8.5;
//ownable.sol
contract Ownable {
address owner;
constructor(){
owner == msg.sender;
}
modifier onlyOwner {
require(msg.sender == owner);
_;
}
}
Destroyable.sol
pragma solidity 0.8.5;
//destroyable.sol
import './ownable.sol';
contract Destroyable is Ownable {
function destroy () public onlyOwner{
selfdestruct(payable(msg.sender));
}
}
helloworld.sol
pragma solidity 0.8.5;
//helloworld.sol
import './destroyable.sol';
contract Bank is Destroyable {
mapping(address => uint) balance;
event depositDone(uint amount, address 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, "Not sufficient!!!");
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, "Balance is not sufficient");
require(msg.sender != recipient, "Transfer failed");
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;
}
}
Jon, That was a fucking(pardon my French) great explanation! thank u , thank u
Hey Ismail,
This is an excellent solution 
The way youâve done it is perfectly valid
Absolutely! But we can also pass msg.sender to selfdestruct(), because the onlyOwner modifier restricts access to the killContract() function to the contract owner, so msg.sender can only ever be the contract owner address anyway.
Again, very observant, and 100% correct We can streamline the inheritance structure by doing exactly what you suggest. This will give you a multi-level inheritance structure.
Just let me know if you have any further questions
Hi @Jimmyjim,
If you pass owner to selfdestruct(), you are right to define the owner state variable as a payable address. But if you have the constructor, and also the onlyOwner modifier in Ownable, you also need the owner state variable in Ownable instead of Destroy. Both the constructor and the modifier reference owner, but if itâs in Destroy it wonât be accessible, because Destroy inherits Ownable, not the other way round. However, if owner is declared in Ownable it will be inherited by Destroy.
The other problem you have is that, even though the remaining funds will be transferred to the contract ownerâs address when the contract is destroyed, any address can call your close() function and trigger selfdestruct() in the first place. This is obviously not very secure! And thatâs why the assignment instructions ask you to restrict access to this function, so that only the contract owner can trigger selfdestruct().
Post your full code for both contracts, otherwise we canât see the full picture. And donât forget you also need to import the files containing the inherited contracts into the files of the derived contracts. We also need to see what modifications youâve made to the start of your Bank.sol file, and Bank contract header, for the inheritance. After all, Bank is the derived contract that we are actually deploying.
Look carefully at the error messages that the compiler generates for you in Remix. The line numbers throwing the errors are highlighted in red, and the error messages give you a good indication of what the problem is and what needs to be corrected.
Let me know if anything is unclear, or if you have any questions
An excellent assignment solution @andrewwy
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.
And well done for realising that you needed to make the owner address payable , so that selfdestruct() is called with a payable address and, therefore, any remaining ether in the contract can be paid/transferred to the owner when selfdestruct() is triggered.
Youâre making great progress! 
An excellent assignment solution and, overall, 3 very well-coded contracts @BShehaj
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.
Youâre making great progress!
Hello Jon,
Thanks for your answer.
I have attached files with full clode from Ownable, destroy and Bank.
First off, I thougt by inherit Ownable the only one who could call the function is the owner, but I assume the caller is always the owner which makes this a problem?
To solve this should I have made the close function as Internal (instead of public as right now)?
Regarding State variable âownerâ , I have this in ownable (OK?):
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
As of now I donât have any errors in remix.
Thank you
Regards
Jimmy
pragma solidity 0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
}
pragma solidity 0.7.5;
import â./Ownable.solâ;
contract Destroy is Ownable {
address payable owner;
function close() public {
selfdestruct(owner);
}
}
pragma solidity 0.7.5;
import â./Ownable.solâ;
import â./destyorable.solâ;
contract Bank is Ownable, Destroy {
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);
msg.sender.transfer(amount); // msg.sender is an address
balance [msg.sender] = 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);
require(msg.sender != recipient, "Don't send money to youself");
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;
}
}
Ownable.sol -
pragma solidity 0.7.5;
contract Ownable{
address owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //run function
}
constructor() {
owner = msg.sender;
}
}
Destroyable.sol -
pragma solidity 0.7.5;
import â./Ownable.solâ;
contract Destroyable is Ownable {
function kill() public {
if (msg.sender == owner)
selfdestruct(msg.sender);
}
}
Bank.sol -
pragma solidity 0.7.5;
import â./Ownable.solâ;
import â./Destroyable.solâ;
contract Bank is Ownable, Destroyable {
âŚ
}
Ownable.sol
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner,'Accessible by owner only');
_;
}
constructor(){
owner = msg.sender;
}
}
Destroyable.sol
pragma solidity 0.7.5;
import './Ownable.sol';
contract Destroyable is Ownable{
function selfDestruct() public onlyOwner{
selfdestruct(owner);
}
}
helloWorld.sol
pragma solidity 0.7.5;
import './Destroyable.sol';
contract Bank is Destroyable{
...
}
After calling selfDestruct(), I noticed the following -
That makes absolutely no sense. How to cope with this behaviour?
I just looked at Filipâs solution.
In the Ownable.sol, he has defined owner with internal visibility.
As far as I know, the default visibility of state variables is internal.
Can somebody explain the variance here?
pragma solidity 0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner{
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
}
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function DESTROY() public onlyOwner {
selfdestruct(msg.sender);
}
}
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract Bank is Destroyable {
Here is my solution:
Base (parent) contract to both Destroyable.sol and Bank.sol
pragma solidity 0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //run the function
}
constructor() {
owner = msg.sender;
}
}
Derived (child) contract to Ownable.sol, but Base (parent) contract to Bank.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function close() public onlyOwner {
//is the caller the contract owner?
assert(msg.sender == owner);
//transfer balance to owner contract
address payable _owner = msg.sender;
//destroy owner contract
selfdestruct(_owner);
}
}
No other code was added to the Bank contract for my solution.
Derived (child) contract to both Ownable.sol and Destroyable.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable {
...
}
Thanks, and hope this is of satisfaction. Please let me know how I can improve or any alterations I require. 
Hi @Jimmyjim,
Your coding of the inheritance structure is good, but there are still several issues remainingâŚ
(1)
You now have the owner variable declared twice â once in Ownable and again in Destroy. This throws a compiler error, and if you look at the error message it will tell you what the problem is. This error makes it impossible to deploy the Bank contract. Did you not try to do this, and find that you couldnât?
The whole point of inheritance is that functions and state variables with either public or internal visibility in the base contract (Ownable) are available in the derived contract (Destroy). So you only need to declare the owner state variable once in Ownable, and as payable. It will be inherited if it has either public or internal visibility. State variables have internal visibility by default, which means they will have internal visibility if you donât explicity state the visibility.
(2)
Once you modify your code for section 1, your code will now compile, and you will be able to deploy Bank. But any address can now call your close() function and trigger selfdestruct(). This is obviously not very secure! And thatâs why the assignment instructions ask you to restrict access to this function, so that only the contract owner can trigger selfdestruct().
No⌠who can or canât call a function will be determined by any restrictions defined in require statements, either within the function body, or applied using a modifier. The purpose of Ownable is to:
So you prevent anyone other than the contract owner from calling close() and triggering selfdestruct(), by applying the onlyOwner modifier to that function. All modifiers in a base contract are inherited by the derived contract, so the onlyOwner modifier in Ownable is available to apply to close() in Destroy.
As well as anyone being able to call close() and destroy the contract, the other problem you will have before making the modification discussed in this section, is that the remaining funds in the contract will not be transferred anywhere when the contract is destroyed, and will be lost. The reason for this is explained in section 3.
(3)
But after modifying your code for section 2, no one will now be able to call your close() function. This is because your constructor has disappeared from Ownable. It is the constructor that assigns the deployerâs address to the owner variable (and therefore sets the contract owner) when the Bank contract is deployed. This happens when Bank is deployed because Bank inherits Ownable.
No constructor means no contract owner address.
No contract owner address means the modifierâs require statement will always fail.
And if the modifierâs require statement always fails, itâs impossible for the close() function to execute and call selfdestruct().
The owner state variable not having an address assigned to it, is also the reason why the remaining funds in the contract were not sent anywhere, and were lost, when anyone could call the close() function.
Once youâve fixed all of these issues, you can check that everything is working as it should by deploying Bank, performing some transactions, then ensuring that only the contract owner address can call close() and destroy the Bank contract and, finally, that the remaining ether balance is transferred to the contract ownerâs external wallet address â you should see their ether balance increase by that amount.
No⌠because if you make close() internal, it will be inherited by Bank, but will only be accessible from within the Bank and the Destroy contracts, and not externally. This means that the contract owner will not be able to call close() to destroy the contract using their own external wallet address (the external address that deployed the Bank contract in the first place).
Let me know if anything is still unclear, and if you have any further questions
Hi @MightyYak,
On the whole, this is a good solution which works
However, you shouldnât use an if statement on its own, in the way you have, to prevent selfdestruct() being triggered by anyone other than the contract owner. If you deploy your Bank contract and then try to call kill() with an address that isnât the contract owner, the contract wonât be destroyed, but you will notice that the transaction is still successful. What we need to happen, instead, is for the transaction to revert, and to be able to handle the error by generating an appropriate error message. You could do this with your if statement as follows:
if (msg.sender != owner) {
revert("Restricted to the contract owner!");
}
selfdestruct(msg.sender);
However, for a while now, Solidity has provided us with this revert functionality already built into the more succinct require() syntax, allowing us to do the same as the above, but in a cleaner and more intuitive way, as follows:
require(msg.sender == owner, "Restricted to the contract owner!");
selfdestruct(msg.sender);
And seeing this require statement should make you realise that we actually already have this restriction available for implementation in the form of our onlyOwner modifier (inherited from Ownable). The point of modifiers is to avoid code duplication, and so adding onlyOwner to your kill() function header is the best way to apply this restriction to your kill() function.
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
An excellent assignment solution @tanu_g
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.
You have also raised some important points concerning one of the disadvantages of using selfdestruct()
As you have pointed out, after selfdestruct() has executed, successful calls can still be made to the destroyed contractâs functions! This is a serious problem if a user isnât informed that the contract has been destroyed and unwittingly sends ether to the contract (e.g by calling the deposit function) because it will be lost. We know that the contract has been destroyed because, as you have also pointed out, getBalance() will return 0 for any user who previously had a positive individual account balance as a result of ether they had deposited in the contract.
As a result of this, if a smart contract deployed on the mainnet is ever destroyed, all users need to be notified that they must not send any more funds to that contract address, otherwise those funds will be lost. There are more secure ways to manage this kind of situation (e.g. by using proxy contracts) but this is out of the scope of this introductory course. If you are interested, then I would highly recommend that you take the Ethereum Smart Contract Security Course, either after this course, or after youâve also done Ethereum Smart Contract Programming 201.
Itâs not necessary to add the internal keyword if you want a state variable to have internal visibility, because as you have quite rightly pointed out state variables have internal visibility by default. However, Solidity currently allows us to still add the keyword internal if we wish, and the code still compiles and performs in exactly the same way. I guess itâs nice to have the ability to highlight that a state variable has internal visibility where this may not be what is expected, and also makes it less likely that the visibility will be changed to a different one during further development.
Just one final minor observationâŚ
I would avoid naming the function which contains selfdestruct() âselfDestructâ as well. Even though the 2 identifiers are different by 1 capital letter, this still risks causing confusion when reading or manipulating your code.
Iâm really impressed with the progress youâve made during this 101 course @tanu_g and Iâm sure you will really enjoy and get a lot out of the more advanced Ethereum Smart Contract courses that follow!
Thank you so much @jon_m for solving all my queries.
I truly appreciate your input. Iâll work hard to refine my coding skills. 
