Inheritance Assignment

jon_m · October 12, 2021, 10:04pm

Nice solution @Jules

No you don’t need to. 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 will still work in exactly the same way if Bank explicitly inherits both Destroyable and Ownable, and if both destroyable.sol and ownable.sol are imported, but it’s unnecessary.

HRMS2021 · October 13, 2021, 12:19am

Bank.sol

pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";

contract Bank is Ownable {

Destroyable.sol

mport "./Ownable.sol";
pragma solidity >0.7.5;

contract Destroyable is Ownable {
 
 function destroy() public onlyOwner { 
  address payabble receiver = msg.sender;
  selfdestruct(receiver);
 }
}

Ownable.sol

pragma solidity 0.7.5;

contract Ownable {
    address public owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;//run the function
    }
    
      constructor(){
        owner = msg.sender;
    }
}

jon_m · October 14, 2021, 5:45pm

Hi @HRMS2021,

There are a few issues here, but they are easily fixed:

(1) The following two lines of code in Destroyable.sol contain misspelt keywords. These generate red compiler errors, but maybe these are just copy-and-paste errors and aren’t in your code in Remix …

(2) Bank can’t be deployed because of the following compiling issue caused by Destroyable.sol …

Imported files must have a compiler declaration that is compatible i.e. both files must be able to be compiled using the same version of Solidity.

Your Ownable.sol can only be compiled using v0.7.5. It cannot be imported into your Destroyable.sol because your Destroyable.sol requires a compiler version of v0.7.6 or above …

Your Destroyable.sol also cannot be imported into your Bank.sol for the opposite reason: your Bank.sol can only be compiled using v0.7.5.

Remix should have generated red compiler errors for this. These errors won’t have highlighted a specific line of code, and so the error messages can’t be viewed by hovering over the red indicator highlighting the line number. However, they will be displayed in the Solidity Compiler panel — the Solidity Compiler icon indicates this with the number of error messages highlighted in red.

(3) Once the above issues are fixed, and your code compiles successfuly, you’ll be able to deploy your Bank contract. However, you’ll notice that the destroy() function is not available for the owner to call. Your selfdestruct functionality is coded correctly, but it’s not inherited by Bank. Can you correct this? When you have, you will be able to deploy Bank, perform some transactions, and then get the contract owner to destroy the Bank contract (by calling the destroy function from Remix). Destroying the contract will also transfer any remaining ether balance to the owner’s external address.

Here, a concise alternative is to call selfdestruct() with msg.sender directly, instead of using the additional receiver variable.

selfdestruct(msg.sender);

Have a look at this post for further details about the use of the additional local variable in the model solution.

Let me know if anything is unclear, or if you have any questions about how to correct your code for these issues

HRMS2021 · October 15, 2021, 12:55am

Thank you @jon_m for your feedback, I will make changes as per your suggestions.

Ruben99 · October 15, 2021, 9:01am

We define the Ownable class

pragma solidity 0.7.5;

contract Ownable {
    
    address internal owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _; //run the function
    }
    
    constructor(){
        owner = msg.sender;
    }
}

In the Destroyable contract we inherit from Ownable so only the owner can destroy the contract.

import "./ownable.sol";

pragma solidity 0.7.5;

contract Destroyable is Ownable{
    
    function destroy() public onlyOwner{
     selfdestruct(payable(msg.sender));
    }
}

Then in the main contract we inherit from destroyable.

import "./destroyable.sol";

pragma solidity 0.7.5;

contract Bank is Destroyable{

jon_m · October 15, 2021, 3:27pm

Great solution @Ruben99

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.

Just one comment…

This works, but you are using Solidity v0.7.5, and prior to Solidity v0.8 msg.sender is already a payable address by default. This means that whenever you want to use msg.sender as a payable address (such as here, as the argument selfdestruct is called with), you don’t have to explicitly convert it e.g.

selfdestruct(msg.sender);

However, from v.0.8 msg.sender is non-payable by default, and so whenever you are using v.0.8 and need to use msg.sender as a payable address, this requires the explicit conversion that you have used.

Let me know if you have any questions.

By the way, you seem to have missed out the Transfer Assignment — is that on purpose because it was too easy?

Keep up the great coding!

vale_dinap · October 19, 2021, 1:09am

Here is my first solution:

First file: ownable.sol

// Base contract - owner functions

contract Ownable{
    address owner;
    
    // constructor
    constructor(){
        owner = msg.sender;
    }
    
    // modifiers
    modifier onlyOwner {
        require(msg.sender == owner, "Only contract owner can do this");
        _;
    }
    
}

Second file: destroyable.sol

// Self destruct contract (owner only)

import "./ownable.sol";

contract Destroyable is Ownable{ 

    // This destroys the contract forever
    function closeContract() public onlyOwner {
        payable(owner).transfer(address(this).balance);
        assert(address(this).balance==0);
        selfdestruct(payable(owner));
    }
    
}

Third file: bank.sol

pragma solidity ^0.8.7;
import "./destroyable.sol";

contract Bank is Destroyable{
    
    // state variables
    mapping (address=>uint) balance;
    
    // events
    event depositDone(uint indexed amount, address indexed depositedTo); //deposit
    event withdrawDone(uint amount, address withdrawnFrom); //withdraw
    
    // functions
    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(amount<=balance[msg.sender], "The amount requested to withdraw exceeds the available balance");
        uint balanceBefore = balance[msg.sender]; //store original balance for later safety checks
        balance[msg.sender]-=amount; //update balance, BEFORE actually transfering ETH in order to avoid attacks
        payable(msg.sender).transfer(amount); //transfer ether
        assert(balanceBefore - amount == getBalance()); // ensure that the actual balance corresponds to the operation performed
        emit withdrawDone(amount, msg.sender); //withdraw event logging
        return balance[msg.sender]; //updateD balance;
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }
    
}

The solution seems to work… HOWEVER

While deploying and testing the smart contract I noticed that, after calling selfDestruct, all of the functions (apart from becoming unusable as desired) accept to be sent any amount of ETH without throwing an error.
Obviously, I want to prevent users to mistakenly send ETH to a contract that has been previously destroyed, as they will never be able get them back.

Since I realized that selfDestruct essentially overrides everything with zeroes, I attempted to amend the contracts by adding:

a function that checks if the contract has been destroyed (by checking if the owner variable is equal to zero);
a modifier that require the contract to be non-destroyed in order to run the functions;
such modifier is applied to all of the functions of the Bank contract.

Here is the amended code:

First file: ownable.sol - (UNCHANGED)
Second file: destroyable.sol

// Self destruct contract (owner only)

import "./ownable.sol";

contract Destroyable is Ownable{

    // This destroys the contract forever
    function closeContract() public onlyOwner {
        payable(owner).transfer(address(this).balance);
        assert(address(this).balance==0);
        selfdestruct(payable(owner));
    }
    
    // This checks if the contract has been destroyed
    function isActive() view internal returns(bool){
        bool result = (owner != address(0));
        return result;
    }
    
    // This prevents functions to be executed on previously destroyed contracts
    modifier onlyActive {
        require(isActive() == true, "The contract has been deactivated, this operation will not be performed");
        _;
    }
}

Third file: bank.sol

pragma solidity ^0.8.7;
import "./destroyable.sol";

contract Bank is Destroyable{
    
    // state variables
    mapping (address=>uint) balance;
    
    // events
    event depositDone(uint indexed amount, address indexed depositedTo); //deposit
    event withdrawDone(uint amount, address withdrawnFrom); //withdraw
    
    // functions
    function deposit() public payable onlyActive returns (uint){
        balance[msg.sender] += msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }

    function withdraw(uint amount) public onlyActive returns (uint){
        require(amount<=balance[msg.sender], "The amount requested to withdraw exceeds the available balance");
        uint balanceBefore = balance[msg.sender]; //store original balance for later safety checks
        balance[msg.sender]-=amount; //update balance, BEFORE actually transfering ETH in order to avoid attacks
        payable(msg.sender).transfer(amount); //transfer ether
        assert(balanceBefore - amount == getBalance()); // ensure that the actual balance corresponds to the operation performed
        emit withdrawDone(amount, msg.sender); //withdraw event logging
        return balance[msg.sender]; //updateD balance;
    }
    
    function getBalance() public view onlyActive returns (uint){
        return balance[msg.sender];
    }
    
}

Yet, even after this change, the unwanted behavior persists!
Is there any way to prevent functions from destroyed contracts to accept ETH? Does my code have any mistake? If not, what is the best workaround?

Assuming that the issue might depend on the nature of the selfDestruct command itself, I attempted another workaround to get a similar functionality without using it at all.
In this case, I am adding a global state variable named “destroyed” (false by default) to check via a modifier if the functions are allowed to run: instead of using selfDestruct, when I want to disable the contract I simply override the value of “destroyed” to true which prevents all functions to be used from that time on.

Here is the code:

First file: ownable.sol

// Base contract allowing owner functions

contract Ownable{
    address owner;
    bool destroyed;
    
    // constructor
    constructor(){
        owner = msg.sender;
        destroyed = false;
    }
    
    // modifiers
    modifier onlyOwner {
        require(msg.sender == owner, "Only contract owner can do this");
        _;
    }
    
}

Second file: destroyable.sol

// Self destruct contract (owner only)

import "./ownable.sol";

contract Destroyable is Ownable{ 

    // This destroys the contract forever
    /*function closeContract() public onlyOwner {
        selfdestruct(payable(owner));
    }
    */
    function closeContract() public onlyOwner isActive {
        payable(owner).transfer(address(this).balance);
        assert(address(this).balance==0);
        destroyed=true;
    }
    
    modifier isActive {
        require(destroyed==false, "This contract has been destroyed and is no longer able to run");
        _;
    }
    
}

Third file: bank.sol

pragma solidity ^0.8.7;
import "./destroyable.sol";

contract Bank is Destroyable{
    
    // state variables
    mapping (address=>uint) balance;
    
    // events
    event depositDone(uint indexed amount, address indexed depositedTo); //deposit
    event withdrawDone(uint amount, address withdrawnFrom); //withdraw
    
    // functions
    function deposit() public payable isActive returns (uint){
        balance[msg.sender] += msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }

    function withdraw(uint amount) public isActive returns (uint){
        require(amount<=balance[msg.sender], "The amount requested to withdraw exceeds the available balance");
        uint balanceBefore = balance[msg.sender]; //store original balance for later safety checks
        balance[msg.sender]-=amount; //update balance, BEFORE actually transfering ETH in order to avoid attacks
        payable(msg.sender).transfer(amount); //transfer ether
        assert(balanceBefore - amount == getBalance()); // ensure that the actual balance corresponds to the operation performed
        emit withdrawDone(amount, msg.sender); //withdraw event logging
        return balance[msg.sender]; //updateD balance;
    }
    
    function getBalance() public view isActive returns (uint){
        return balance[msg.sender];
    }
    
}

Everything seems to work exactly as I expected. In addition, this also allows choosing the individual functions that are meant to become unusable (as, I guess, one might leave some view functions still accessible for some reasons), instead of destroying the full functionality of the smart contract.

Is there any reason NOT to use a global state variable to disable the contract as I just did?

(the only reason that comes to my mind is that the owner might still have the ability to re-activate the contract, but this would be solved by making the closeContract() function also permanently override the owner address with the “blackhole” ethereum address 0x0000000000000000000000000000000000000000, so that nobody will ever be able to regain control of the smart contract… is that right?)

Many thanks!

Langren · October 19, 2021, 12:14pm

new try --------------------------

Langren · October 19, 2021, 12:41pm

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 close() public {
selfdestruct(msg.sender);
}
}

pragma solidity 0.7.5;

import “./selfdestruct.sol”;

contract Bank is Destroyable {
mapping(address => uint) balance;
event depositDone(uint amount, address indexed depositedTo);
event transferFromToAmount(uint amount, address indexed transferredFrom, address indexed transferredTo);

function deposit() public payable returns (uint) {
balance[msg.sender] += msg.value;
emit depositDone(msg.value, msg.sender);
return balance[msg.sender];
}

function getBalance() public view returns(uint){
return balance[msg.sender];
}

function withdraw(uint amount) public returns (uint) {
//msg.sender is an address
//address payable toSend = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
//toSend.sender.transfer d.h. msg.sender entspricht address payable
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, “Dont transfer money to yourself”);

   uint previousSenderBalance = balance[msg.sender];
   
  _transfer(msg.sender, recipient, amount);
  
  assert(balance[msg.sender] == previousSenderBalance - amount);
  
  emit transferFromToAmount(amount, msg.sender, recipient);
  
   //event logs and further checks-->

}

function _transfer(address from, address to, uint amount) private {
balance[from] -= amount;
balance[to] += amount;
}
}

jon_m · October 20, 2021, 12:48am

This is an excellent analysis @vale_dinap

Your first solution is correct for an implementation of selfdestruct() using inheritance, except that there is no need to add the following two lines of code to your closeContract function in Destroyable …

Your code works with these two additional lines, and from a technical point of view they are well coded. However, as well as destroying the contract, selfdestruct() automatically transfers the remaining contract balance to the address argument it is called with. This is all part of the pre-defined selfdestruct() functionality. If you remove these two lines of code, you will see that calling the closeContract() function produces exactly the same results.

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

Using selfdestruct() brings with it certain clear disadvantages, the potential repercussions of which need to be fully understood before choosing to implement it. As you have correctly identified, 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, otherwise, and as you have also discovered, those funds will be lost.

Yes … the additional code you’ve added will have no effect whatsoever because, even though function calls can still be made, the contract itself has actually been destroyed and removed from the blockchain. Calling selfdestruct() doesn’t somehow overwrite all of the values stored in the contract state with zero values, because, from that point, the contract state ceases to exist. The zeros that are returned and, more seriously, any ether that is sent and lost, are just unwanted side-effects of making external calls to a destroyed (and now non-existant) contract.

Having said that, however, your experimentation has led you to a very interesting and well thought-out “workaround”. Your 3rd solution actually ends up implementing something similar to what is called the pausable pattern. The following blog post sets out and explains the reasons why a pausable contract has many advantages over the more “primitive” selfdestruct

https://blog.b9lab.com/selfdestruct-is-a-bug-9c312d1bb2a5

Your variation of the pausable pattern results in a more extreme, permanent and irreversible “pause”, whereas, more generally, pausable contracts are able to provide a range of more nuanced solutions, where…

an authorised address can both pause and unpause a contract; and
by using two different modifiers, specific functions can be activated or deactivated depending on whether the contract is currently paused or unpaused.

In your concluding comments you do show that you have already started to think along these lines …

OpenZeppelin Contracts (see their GitHub repository, and README file) provides a smart contract library which includes a “ready-made” Pausable contract:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.3.0/contracts/security/Pausable.sol

The functionality provided by OpenZeppelin in this library can be implemented by inheriting the relevant contracts.

In terms of your 3rd solution, here are some additional comments and observations about certain aspects…

No … you will see from the code of the OpenZeppelin Pausable Contract that you have the right idea using…

…setting it to false on deployment with a constructor …

… and enabling it to be set to true by an authorised address via a separate function.

However …

Even though when destroyed is set to true your contract will effectively become unusable, it will remain deployed. So, I think a more suitable term than destroyed would (i) make a better name for the state variable, and (ii) be more suitable for the isActive modifier’s error message;
In order to keep the contract ownership and contract destruction/deactivation functionalities within separate contracts, it would be better to put the bool state variable in Destroyable, together with a separate constructor. This keeps the code more modular and reusable.

The contract owner address isn’t able to re-activate your contract after calling closeContract(). For that to be possible, you would need to add an addtional function which would remain active after the bool state variable has been set to true e.g.

function reactivate() public onlyOwner {
    require(destroyed, "The contract is already active");
    destroyed = false;
}

This is correct, and would work if the owner was otherwise able to re-activate the contract; but as I’ve explained above, this is impossible with your current 3rd solution. In addition, as discussed in the blog post, and mentioned above, there are good reasons why we may not want to prevent re-activation of the contract anyway.

Even though using selfdestruct() has clear disadvantages, for the purposes of this introductory course it provides a simple and straightforward way to practise implementing inheritance, which is our main objective in this assignment. Pausable contracts involve more advanced techniques and are therefore out of the scope of this introductory course. So, don’t worry if you don’t fully understand everything discussed in the blog post, or all of the information and code in the other links to OpenZeppelin Contracts. You will learn about pausable contracts and other smart-contract emergency and security features, such as proxy contracts, in the Smart Contract Security course, which I highly recommend you take either after this one, or after the Ethereum Smart Contract Programming 201 course. The 201 course will also introduce more advanced concepts and techniques which will also help you to more fully understand and work effectively with this material.

As always, just let me know if you have any further questions.

You’re making great progress! Keep up the great work

jon_m · November 12, 2021, 11:53pm

Hi @Langren,

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.

Your inheritance structure makes the selfdestruct() functionality in Destroyable available within Bank when Bank is deployed. When close() is called, the Bank contract will be successfully destroyed and any remaining ether in the contract will be transferred to the contract owner’s external address.

However, even though any remaining ether in the contract will only be transferred to the contract owner, at the moment, anyone can call close(), trigger selfdestruct(), destroy the contract, and trigger this transfer in the first place. The payable address passed to selfdestruct() determines where the remaining funds will be transferred to, but it doesn’t restrict who can trigger selfdestruct().

So, to the extent that the contract owner can be relied upon to be a trustworthy custodian, the remaining funds are secure; but the fact that anyone can destroy the contract is still a huge problem. So that’s why, as well as ensuring that the address passed to selfdestruct() is a secure destination (from where the remaining funds can be appropriately managed), we also need to restrict who can trigger selfdestruct(). Part of the assignment is to ensure that only the contract owner can destroy the contract. We already have an onlyOwner modifier in Ownable which is inherited by Destroyable, so you can just apply onlyOwner to close() without having to define the modifier again in Destroyable.

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

Just let me know if you have any questions

Langren · October 21, 2021, 6:11am

Hi Jon, thanks for your feedback. I will check my code to restrict the selfdestruct. Also thanks for the input about formatting the code here in the forum, did not know.
Have a great day!

jon_m · October 22, 2021, 12:07am

You’re very welcome @Langren,
… glad you found the feedback helpful

If you would like any modifications you make checked, then just post your revised code and I’ll take a look for you.

JJ_FX · October 22, 2021, 7:02pm

Ok, this took me a while. I had to rewrite the logic for the Bank, so it makes more sense.

So now when the Bank owner closes the bank each Customer receives back the money they deposit. So this way it’s not a scam Bank

I used mapping(uint => struct) customers that helped me with debugging. But on the other hand it forced looping through. Also I changed to 0.8.9 compiler, because I had some errors on 0.7.5 I couldn’t resolve.

Bank.sol:

// SPDX-License-Identifier: UNLICENCED

pragma solidity 0.8.9;

import “./Academy-Ownable.sol”;
import “./Academy-Destroyable.sol”;

contract Bank is Ownable, Destroyable {

mapping(uint => Customer) customers;
uint public customersCount;

struct Customer {
    address account;
    uint balance;
    uint id;
}

// Register with the bank first. You don't have to deposit upon registering yet
function registerAccount() public payable {
    bool newCustomer = true;
    for (uint i=0; i<customersCount; i++) {
        if (customers[i].account == msg.sender) {
            newCustomer = false;
        }
    }
    
    if (newCustomer) {
        customers[customersCount - 1] = Customer(msg.sender, msg.value, ++customersCount);
    }
}

// Once registered you can now deposit
function deposit() public payable {
    for (uint i=0; i<customersCount; i++) {
        if (customers[i].account == msg.sender) {
            customers[i].balance += msg.value;
            break;
        }
    }
}

function getBalance() public view returns (uint) {
    for (uint i=0; i<customersCount; i++) {
        if (customers[i].account == msg.sender) {
            return customers[i].balance;
        }
    }
    
    return 0;
}

function withdraw(uint _amount) public payable {
    for (uint i=0; i<customersCount; i++) {
        if (customers[i].account == msg.sender) {
            require(customers[i].balance >= _amount);
            payable(msg.sender).transfer(_amount);
            customers[i].balance -= _amount;
            break;
        }
    }
}

function closeBank() public mOwnerOnly {
    for (uint i=0; i<customersCount; i++) {
        payable(customers[i].account).transfer(customers[i].balance);
    }
    
    permanentlyCloseBank();
}

}

Ownable.sol:

// SPDX-License-Identifier: UNLICENCED

pragma solidity 0.8.9;

contract Ownable {
    
    address public owner;
    
    constructor() {
        owner = msg.sender;
    }
    
    modifier mOwnerOnly {
        require(msg.sender == owner);
        _;
    }
}

Destroyable.sol:

// SPDX-License-Identifier: UNLICENCED

pragma solidity 0.8.9;

import "./Academy-Ownable.sol";

contract Destroyable is Ownable {
    
    function permanentlyCloseBank() internal {
        selfdestruct(payable(owner));
    }
}

Lennart_Lucas · October 22, 2021, 8:46pm

Bank inherits ownable through destructible like so:

pragma solidity 0.7.5;

import "./Destructible.sol";

contract Bank is Destructible{

The code for destructible itself:

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destructible is Ownable{
    
    function destroyContract() public onlyOwner {
        selfdestruct(msg.sender);
    }
}

zlr1984 · October 24, 2021, 12:22pm

Ownable.sol

pragma solidity 0.8.7;

contract Ownable {

address payable owner;

 modifier onlyOwner {
    require (msg.sender == owner);
    _; // run the function
}

constructor () {
    owner == msg.sender;
}

}

Destroyable.sol

pragma solidity 0.8.7;

import “./Ownable.sol”;

contract Destroyable is Ownable {

function close() public onlyOwner { 
    selfdestruct(owner); 
}

}

Bank.sol

pragma solidity 0.8.7;

import “./Destroyable.sol”;

contract Bank is Destroyable {

josejalapeno · October 24, 2021, 1:51pm

I am so glad to have finally arrived at my final answer, thank God I am really getting this, I am sorry for the deleted replies. Please verify if my code is true, I really hope to fully understand programming and to also write on my own in the future and to also make a living out of it.

So here’s the code:
Bank Contract:

pragma solidity 0.7.5;

import "./own.sol";
import "./Destroyable.sol";

contract Bank is Own, Destroyable

Own Contract: (changed it to own, cause there’s already an ownable JSON so I think its auto filling it)

pragma solidity 0.7.5;


contract Own {
    address payable public owner;
    modifier onlyOwner{
        require(msg.sender == owner);
        _;
    }
    constructor(){
        owner = msg.sender;
    }
}

Destroyable Contract:

pragma solidity 0.7.5;

import "./own.sol";

contract Destroyable is Own{

    function close() public onlyOwner {
    selfdestruct(owner);
    }
}

jon_m · October 24, 2021, 5:29pm

Great solution @Lennart_Lucas

Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destructible because, as you say, it will implicitly inherit Ownable via Destructible…

Keep up the great coding!

jon_m · October 24, 2021, 8:25pm

Hi @zlr1984,

You have a couple of issues here, which are preventing your code from working as it should…

(1)

Your constructor in Ownable needs to assign msg.sender to the owner state variable, not compare them as it does here …

At the moment, no one can call selfdestruct(), because the owner state variable hasn’t been assigned an address. So calling close() will always trigger revert and throw an error.

(2)

Prior to Solidity v0.8 msg.sender is already a payable address by default. This means that whenever you want to use msg.sender as a payable address you don’t have to explicitly convert it. For example, in Ownable, your owner state variable is declared with a payable address type. With Solidity v0.7 the constructor can assign msg.sender to owner without having to explicity convert it.

// Solidity v.0.7

address payable owner;

constructor() {
    owner = msg.sender;
}

However, from v.0.8 msg.sender is non-payable by default, and so whenever you are using v.0.8 and need to use msg.sender as a payable address, this requires an explicit conversion …

// Solidity v.0.8.7

address payable owner;

constructor() {
    owner = payable(msg.sender);
}

Once you have corrected the operator in the constructor, you will get a red compiler error for this second issue.

What similar change do you have to make in your Bank contract when you use v0.8 instead of v0.7 ?
(Hint: you need to make an additional modification to your solution to the Transfer Assignment)

Everything else is well coded. 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

Let me know if anything is unclear, or if you have any questions

jon_m · October 24, 2021, 8:44pm

This is a very good solution @josejalapeno

You’re making great progress!

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, as follows:

own.sol

contract Own { ... }

Destroyable.sol

import "./own.sol";
contract Destroyable is Own { ... }

Bank.sol

import "./Destroyable.sol";
contract Bank is Destroyable { ... }

Just let me know if you have any questions