Inheritance Assignment

[jak]

To be honest, I’m not sure if the code is correct - if someone wants to comment, I would greatly appreciate.

The reason I’m not sure is correct is that when I check ‘admin’ balance after the contract selfdestructs, the balance becomes 0 - effectively all balance is removed.
I’m not sure if this is what to expect.

Thanks in advance, here is my code nonetheless:

Destroyable.sol file

import "./ownable.sol";

contract Destroyable is Ownable{
    
    function close() public onlyOwner { 
        address payable admin = msg.sender;
        selfdestruct(admin); 
 }
    
}

Ownable.sol file

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

Bank Header Changes

pragma solidity 0.7.5;

import "./ownable.sol";
import "./destroyable.sol";

contract Bank is Ownable, Destroyable {

[TuomoP]

pragma solidity 0.8.4;

import "./Ownable.sol";

contract Destroyable is Ownable {
    
    function destroy() public onlyOwner {
        address receiver = msg.sender;
        selfdestruct(msg.sender);
        //payable(msg.sender).transfer(amount);

        
    }
}

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

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

[FerfiC]

So we can start by the Ownable.sol:

pragma solidity 0.8.4;

contract Ownable {

address public owner;

modifier onlyOwner {
   require (msg.sender == owner);
   _; 

}

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

An then the Destryable.sol

pragma solidity 0.8.4;

import “./Bank.sol”;
import “./Ownable.sol”;

contract Destroyable is Ownable,Bank {

address payable addr = payable(address(owner));

function deleteAddress () public payable onlyOwner {
    if(balance[msg.sender] > 0){
       withdraw(balance[msg.sender]);
    }
    selfdestruct(addr);
}

}

And the Bank.sol

pragma solidity 0.8.4;

import “./Ownable.sol”;

contract Bank is Ownable {

[MattPerry]

import "./Ownable.sol";

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

[thecil]

Hey @DavidV, hope you are ok.

I did not understand quite well your question. the bank contract inherits Ownable and Destroyable so it can access to their functionalities.

Carlos Z

[thecil]

@jak

The reason I’m not sure is correct is that when I check ‘admin’ balance after the contract selfdestructs, the balance becomes 0 - effectively all balance is removed.

After you destroy the contract, the remining funds will be transfered to admin, so you should check not the balance of the owner from the contract, instead check the ether balance on remix (which should increase after the contract is destroyed)

Carlos Z

[DavidV]

My question was this: if bank inherits destroyable and destroyable inherits ownable, does bank still need to inheritOwnable as well?

[cgironda]

The destroyable.sol file is:

pragma solidity 0.7.5;

import "./ownable.sol";

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

I did not change the ownable.sol original file, but I am sure something must be changed, otherwise, the instructions to solve this problem wouldn’t have mention it. (I did not check the solution yet)

In the inherit file I just added the word in Destroyable at the header of the contract Bank:

contract Bank is Ownable, Destroyable {
    // code here
}

[Bitborn]

Destroyable.sol file…

import “./Ownable.sol”;
pragma solidity 0.7.5;

contract Destroyable is Ownable {

function terminate() public onlyOwner {
address payable receiver = msg.sender;
selfdestruct(msg.sender);
}

Ownable.sol file

contract Ownable {
address internal owner;

modifier onlyOwner {
    require(msg.sender == owner);
    _; 
}

constructor(){
    owner = msg.sender;
}

}
Bank Header Changes

pragma solidity 0.7.5;

import “./ownable.sol”;
import “./destroyable.sol”;

contract Bank is Ownable, Destroyable {

[Nathan_Burkiewicz]

// ownable.sol
pragma solidity 0.7.5;

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

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

// destroyable.sol
pragma solidity 0.7.5;

import "./ownable.sol";

contract Destroyable is Ownable {
    function destroy() public onlyOwner { 
        selfdestruct(owner); 
    }
}

// bank.sol
pragma solidity 0.7.5;

import "./ownable.sol";
import "./destroyable.sol";

contract Bank is Ownable, Destroyable {
//
}

[thecil]

@DavidV

if bank inherits destroyable and destroyable inherits ownable, does bank still need to inheritOwnable as well?

It is has you said, simple explenation:

You have 3 contracts.
IF contract 1 IS contract 2 AND contract 2 IS contract 3.
Contract 1 will have access to contract 3, without needing to specify that contract 1 is contract 3.

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.1;

import "./ContractTwo.sol";

contract ContractOne is ContractTwo {
    
    function callOne(uint _num) public pure returns(uint){
        require(_num > 0 && _num <= 3, "_num Should be higher or above 3");
        if(_num == 2){
           return callTwo(); 
        }
        else{
            return callThree(); 
        }
    }
}

// SPDX-License-Identifier: UNLICENSED

pragma solidity 0.8.1;

import "./ContractThree.sol";

contract ContractTwo is ContractThree {
    function callTwo() internal pure returns(uint){
        return 2;
    }
}

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.1;

contract ContractThree {
    function callThree() internal pure returns(uint){
        return 3;
    }
}

Carlos Z

[Julian97]

helloworld.sol (beginning):

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 public owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _; // run the function
    }
    
        constructor(){
        owner = msg.sender;
    }
}

Destroyable.sol:

pragma solidity 0.7.5;

import "./Ownable.sol";

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

[Filip_Rucka]

pragma solidity 0.7.5;

contract Ownable {
    
    address payable private owner;
    
    
       constructor(){
         owner = msg.sender;
         
    }
     modifier onlyOwner{
        require(msg.sender == owner);
        
        _;
    }
    
 
}
pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {
    
address payable private owner;
 
 function close() public { 
  selfdestruct(owner); 
 }
}

[jon_m]

An excellent assignment solution @mvrpraveen

Apologies for the delay in providing you with some feedback!

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!

[jon_m]

An excellent solution @DavidV
… and some very good questions…

@DavidV

why does the Bank code need to inherited both Ownable and Destroyable? Ownable is already a child of Ownable? Or am I missing how this works. My toe worked fine without Ownable…

@DavidV

My question was this: if bank inherits destroyable and destroyable inherits ownable, does bank still need to inheritOwnable as well?

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.

Notice the distinction here between (i) the code that needs to be inherited by Bank (both Ownable and Destroyable), and (ii) the inheritance relationship that needs to be explicitly declared for that to happen (Destroyable inherits Ownable, and Bank inherits Destroyable). Or another way to think about it is that, if Destroyable inherits Ownable, then Bank only needs to inherit Destroyable, because the Destroyable functionality that Bank will inherit already includes Ownable’s inherited functionality.

---

@DavidV

// BTW why could I not use “owner” in the Selfdestruct function?

The address argument passed to selfdestruct() needs to be a payable address, in order for the contract’s remaining ether balance to be transferred to that address when the contract is destroyed. To receive ether an address must be payable. There are several ways we can code this:

(1) By doing what you’ve done and using msg.sender to reference the contract owner’s address, because this is the only address that it can ever represent in the close() function (as a result of the onlyOwner modifier restricting who can access this function in the first place). Prior to Solidity v.0.8, msg.sender is payable by default, and so that’s why you didn’t need to explicitly convert it…

@DavidV

selfdestruct(msg.sender);

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 e.g.

selfdestruct(payable(msg.sender));

(2) You can pass owner  to selfdestruct(), but the owner state variable has been declared as a non-payable address (the default address type) in Ownable. This means that, to use it as the argument in selfdestruct(), it would need to be converted to a payable address. One way to do this is to declare the owner state variable in Ownable with a payable address type …

address payable public owner;

You will then be able to use owner as the argument in selfdestruct() …

selfdestruct(owner);

(3) If you only need the contract owner’s address to be payable for the purposes of executing selfdestruct(), you can leave the owner state variable as non-payable, and explicitly convert the address to payable locally, within the function where you actually need to use it as payable…

function close() public onlyOwner {
    address payable receiver = payable(owner);
    selfdestruct(receiver);
}

But you can also do this directly within the selfdestruct() function call itself, which I think is more straightforward and concise …

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

Let me know if you have any further questions.

You’re making great progress!

[jon_m]

Nice Destroyable contract, @jtb

As HelloWorld is the contract being deployed, can we also see what changes you’ve made to HelloWorld so that it inherits the necessary functionality?

[jon_m]

Hi @jak,

Your solution “works”, in that…

• Bank inherits all of the necessary functionality from the other 2 contracts;
• after deploying Bank, and performing some transactions, if close() is called, and selfdestruct() triggered, the contract balance is transferred to the contract owner’s external wallet address;
• only the contract owner’s address (set on deployment) can call close() and destroy the contract.

However, the address state variable admin you’ve declared in Ownable, is redundant. It never even stores an address, because one isn’t assigned to it. The local variable (also named admin) which you’ve declared within close(), temporarily stores the msg.sender address, which can only ever be the contract owner’s address, because the onlyOwner modifier restricts who can access this function in the first place. So, by passing this local variable (admin) to selfdestruct() you are correctly calling selfdestruct() with a payable address, which is the contract owner’s address. Just as @thecil has explained to you, you will see the transferred funds added to the contract owner’s external wallet-address account-balance, which you will find in the Account field in Remix.

@thecil

instead check the ether balance on remix (which should increase after the contract is destroyed)

@jak

when I check ‘admin’ balance after the contract selfdestructs, the balance becomes 0 - effectively all balance is removed.
I’m not sure if this is what to expect.

What you’re trying to do here doesn’t make any sense, because, your admin state variable is defined to store an address and not an unsigned integer. And, as I’ve mentioned above, no value is ever assigned to it anyway. Calling selfdestruct() removes the contract code and its data from the EVM, and so any data previously stored in the contract state, including the contract’s ether balance itself, will be lost. That’s why, as part of the selfdestruct() operation, it’s important to transfer any remaining funds to a trusted address. And this is also the reason why any getter you call after the contract has been destroyed will return a zero value.

As msg.sender in the close() function references the contract owner’s address, and because prior to Solidity v.0.8, msg.sender is payable by default, you can simplify your code and pass msg.sender to selfdestruct() directly…

selfdestruct(msg.sender);

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 e.g.

selfdestruct(payable(msg.sender));

An alternative is to pass owner to selfdestruct(), but the owner state variable has been declared as a non-payable address (the default address type) in Ownable. This means that, to use it as the argument in selfdestruct(), it would need to be converted to a payable address. One way to do this is to declare the owner state variable in Ownable with a payable address type …

address payable public owner;

You will then be able to use owner as the argument in selfdestruct() …

selfdestruct(owner);

However, if you only need the contract owner’s address to be payable for the purposes of executing selfdestruct(), you can leave the owner state variable as non-payable, and explicitly convert the address to payable locally, within the function where you actually need to use it as payable…

function close() public onlyOwner {
    address payable receiver = payable(owner);
    selfdestruct(receiver);
}

But you can also do this directly within the selfdestruct() function call itself, which I think is more straightforward and concise …

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

---

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 anything is still unclear, or if you have any further questions

[jon_m]

Hi @TuomoP,

Apologies for the delay in giving you feedback on your solution to this assignment.

As you have chosen to use Solidity v0.8, the compiler should have highlighted a couple of issues with your code…

The address argument passed to selfdestruct() needs to be a payable address, in order for the contract’s remaining ether balance to be transferred to that address when the contract is destroyed. To receive ether an address must be payable.
Prior to Solidity v.0.8, msg.sender is payable by default, and so if you’d used v.0.7, the following line of code would have been correct…

@TuomoP

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 an explicit conversion e.g.

selfdestruct(payable(msg.sender));

This is why the compiler throws an error for this line of your code.

Once you’ve correct this, the compiler will now generate an orange/yellow warning for the previous line of code:

@TuomoP

address receiver = msg.sender;

This is because you have defined a local variable reciever , but not used it! So, you should remove this, as it’s not needed. You could declare a local variable with a payable address type, and then pass that to selfdestruct(), but you would need to do that as follows…

address payable receiver = payable(msg.sender);
selfdestruct(receiver);

---

If you pass msg.sender (converted to a payable address) to selfdestruct(), there is no need to store the contract owner’s address as a payable address in the owner state variable in Ownable.

You are currently converting msg.sender to a payable address within the constructor. But this has no effect (and can be removed) because this address is being assigned to the owner state variable which is defined with the default non-payable address type.

An alternative to passing msg.sender to selfdestruct() is to pass owner instead. To do that it would need to be converted to a payable address. One way to do this is to declare the owner state variable in Ownable with a payable address type …

address payable owner;

… in which case you would need to convert msg.sender to a payable address within the constructor.

You would then be able to use owner as the argument in selfdestruct() …

selfdestruct(owner);

However, if you only need the contract owner’s address to be payable for the purposes of executing selfdestruct(), you can leave the owner state variable as non-payable, and explicitly convert the address to payable locally, within the function where you actually need to use it as payable…

function close() public onlyOwner {
    address payable receiver = payable(owner);
    selfdestruct(receiver);
}

But you can also do this directly within the selfdestruct() function call itself, which I think is more straightforward and concise …

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

---

Can we also 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.

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

[jon_m]

Hi @FerfiC,

Apologies for the delay in providing you with feedback on your solution to this assignment.

You are partly there… but you’ve made things much more complicated than they need to be…

(1) The objective of the assignment is to be able to deploy Bank, perform some transactions, and then have the contract owner destroy the Bank contract. This means that you need a different inheritance structure. Can you work out what that is?
Inherited contracts are usually ones which contain higher-level functionality which is required across a wide-range of different derived contracts. Derived contracts usually contain much more specific functionality, each for its own particular use case, but any common functionality that they share with other derived contracts can be parcelled off into separate base contracts. These base contracts can then be inherited on an “as-needs” basis by those derived contracts which require their high-level functionality to be made available to them.

(2) When selfdestruct() is triggered with its required payable-address argument, it will not only destroy the contract, but it will also transfer any remaining ether held in the contract address balance to the same external address it is called with. That is the purpose of the required payable-address argument. So, the additional if-statement you’ve added to your deleteAddress function is unnecessary…

@FerfiC

if(balance[msg.sender] > 0){
    withdraw(balance[msg.sender]);
}

In actual fact, the code executed, when the if statement’s condition evaluates to true, will not transfer the entire contract balance to the contract owner (the total of all individual account holder balances), but only the contract owner’s individual account balance recorded in the mapping. I’m sure you can see that these balances are not the same.
You may think that the funds belonging to all of the other individual account holders will still be transferred to the contract owner by selfdestruct(), anyway. But point (3) explains why this also won’t happen…

(3) The addr variable defined in this line of code in Destroyable …

@FerfiC

address payable addr = payable(address(owner));

… holds a zero address, and not the contract owner’s address. You can see this for yourself if you make addr public, and then call its ready-made getter in Remix, which Solidity automatically makes availiable for all variables we define with public visibility. Can you work out why it doesn’t hold the contract owner’s address, which is assigned by the constructor to the owner state variable in Ownable on deployment?
As selfdestruct() is called with a payble zero address it will still execute and destroy the contract, but any remaining ether will be transferred to the zero address and lost.
Can you modify this, so that the contract owner’s address is passed to selfdestruct() as a payable address?

(4) deleteAddress() shouldn’t be marked as payable . A function only needs to be payable when it receives ether from an external address in order to update the contract balance. That’s why we marked our deposit function in Bank as payable, but not our withdraw function.

Let me know if you need any help in making these modifications, or if you have any questions

[jon_m]

Nice Destroyable contract, Matthew

As Bank is the contract we are deploying, can we also see what changes you’ve made to Bank so that it inherits the necessary functionality?