Inheritance Assignment

:muscle: :muscle: That’s a great idea.

Absolutely!.. nearly all of the videos are re-recorded, and some of the assignments are quite different. This variety in the material, a few changes in syntax for v.0.7, but the fact we are still covering all of the same concepts and techniques, will mean if you do it immediately after this one, it will be a great way to review and consolidate what you’ve being learning in this course, while still being engaging and not too repetitive. I would highly recommend you do that before moving on to Ethereum Smart Contract Programming 201 :slight_smile:

This Ownable contract will now work for Method 3, Donnie :ok_hand:
You have already made the correct changes to the constructor and the owner state variable (which I mentioned in the first of my replies to you today).

By making the address stored in the owner state variable payable, you don’t need to perform any of the local conversions which we’ve been discussing for Method 1. So, in the close() function (in Destroyable, not Ownable) you can just go ahead and call selfdestruct() with owner like you did here:

But, there is still one error in this code: the wrong file has been imported. Look again, at what I have explained in this post, and get your import statements right at the top of both remaining files.
Also, as I’ve mentioned before, you don’t need to make the close() function payable…

If you do this correctly, then you won’t need to remove onlyOwner like you’ve said here…

This line won’t throw an error if you import the correct files into this file. And, obviously, you must keep your spelling consistent…

When you’ve got that working, you can then experiment with removing payable from the owner state variable in Ownable, and using Methods 1 & 2 to ensure the owner address is payable for selfdestruct() — I’ve explained how to get Method 1 right by using the code in Destroyable, in this post).

Hey @jon_m and thanks for reviewing my code,

I have just checked the changelogs of v0.8 and it says:

Address literals have the type address instead of address payable . They can be converted to address payable by using an explicit conversion, e.g. payable(0xdCad3a6d3569DF655070DEd06cb7A1b2Ccd1D3AF) .

So basically this means if you just use a normal address like this:

address a = 0xdCad3a6d3569DF655070DEd06cb7A1b2Ccd1D3AF;

by default it is NOT payable since v0.8 but can be explicitly converteted like this:

address payable a = payable(0xdCad3a6d3569DF655070DEd06cb7A1b2Ccd1D3AF);

Have I understood this correctly?

Here is the code for my Bank. Since Destroyable is inheriting from Ownable and EtherBank is inheriting from Destroyable, I don’t need my EtherBank to inherit from Ownable, right?
EtherBank.sol

pragma solidity ^0.8.4;

import "./Ownable.sol";
import "./Destroyable.sol";
import "./Government.sol";

interface GovernmentInterface {
    function addTransaction(address _from, address _to, uint _amount) external payable;
    
}

contract EtherBank is Destroyable {

    GovernmentInterface GovernmentInstance = GovernmentInterface(0xC5862Ba753F3DabC4120E9803c01e0B13D7905F2);
    
    mapping(address => uint) balances;
    
    
    event deposited(address sender, uint amount);
    event withdrawn(address withdrawer, uint amount);
    event transfered(address sender, address receiver, uint amount);


    modifier checkFunds(uint requiredAmount){
        require(balances[msg.sender] >= requiredAmount, "Not enough funds for transaction!");
        _; //runs the function
    }
    

    modifier checkValueForZero(uint value){
        require(value > 0, "Value needs to be greater than 0.");
        _;
    }
    
    function deposit() public payable checkValueForZero(msg.value) returns(uint) {
        uint balanceBefore = balances[msg.sender];
        balances[msg.sender] += msg.value;
        emit deposited(msg.sender, msg.value);
        assert(balances[msg.sender] == balanceBefore + msg.value);
        return balances[msg.sender];
    }
    
    function withdraw(uint amount) public payable checkFunds(amount) checkValueForZero(amount) returns(uint){
        uint balanceBefore = balances[msg.sender];
        balances[msg.sender] -= amount;
        payable(msg.sender).transfer(amount);
        emit withdrawn(msg.sender, amount);
        assert(balances[msg.sender] + amount == balanceBefore);
        return balances[msg.sender];
    }
    
    function getBalance() public view returns(uint){
        return balances[msg.sender];
    }
    
    function transfer(address recipient, uint amount) public {
        uint previousSenderBalance = balances[msg.sender];
        _transfer(msg.sender, recipient, amount);
        GovernmentInstance.addTransaction{value: 1 gwei}(msg.sender, recipient, amount);
        emit transfered(msg.sender, recipient, amount);
        assert(balances[msg.sender] == previousSenderBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private checkFunds(amount){
        balances[from] -= amount;
        balances[to] += amount;
    }
}
1 Like

Hey Jon,
I you have put a lot of time into helping me with this. I appreciate it. I am still getting an error with helloWorld. here is the relevant code:

import "./Ownable.sol";
pragma solidity 0.5.12;

contract HelloWorld is Ownable, Destroyable {
import "./Ownable.sol";

pragma solidity 0.5.12;

contract Destroyable is Ownable {

    function close() public payable onlyOwner{
         selfdestruct(owner);
     //option 1
     //address payable receiver = address(uint160(owner));
     //  selfdestruct(receiver);
    // option2
   // function close() public onlyOwner {
  //  selfdestruct(address(uint160(owner)));
     //  selfdestruct(Owner);
       
    }

}

What am I missing???

1 Like

This is working but is there a standart way to make owner address payable?

contract Destroyable {
    address payable owner;
    
    constructor() {
        owner = payable(msg.sender);
    }
    
    function destroy() internal {
        require(msg.sender == owner);
        selfdestruct(owner);
    }
}
1 Like

Your code is now looking good @Michael_Gerst :ok_hand:

Well done for perservering with this :muscle: Hopefully you’ve learnt a lot in the process :slight_smile:

Corrected :white_check_mark:

Corrected, by defining owner as a payable address in Ownable :white_check_mark:

The only thing you haven’t modified is …

To use the onlyOwner modifier defined in Ownable (and inherited by Destroyable and Bank), you just need to add the name onlyOwner to the function header.

Also, have a look at this post which explains why you need to make an important security modification to the order of the statements within your withdraw function body.


This is probably caused by something related to calling the external Government contract function addTransaction. When you say you are getting a revert error when compiling, do you actually mean you are getting a transaction revert error when you call transfer() after you’ve already compiled and deployed your contract? Compiler errors are the ones you get before deployment, and are indicated by a red mark over the line number. Your code should now be compiling without any errors, and your code for this assignment should now work. So try removing all of the code which you’ve added afterwards for the lessons on external contracts: the Government interface; declaration of the governmentInstance variable; and the call to addTransaction in transfer(). You should find that your transfer function now works. That will also prove to you that the revert is thrown by an issue with the external function call.
If that’s the case, then have a look at the discussion topic related to Inheritance & External Contracts, and you’ll probably find the solution there. I suspect that this could be answer.

If this doesn’t resolve the problem, then post any further questions about the Government external contract in the discussion post related to that. But let us know here if you have any more questions about the Inheritance assignment :slight_smile:

Hey @xela,

Correct…this gives you a multi-level inheritance structure. It also means you don’t need to import Ownable.sol into EtherBank’s .sol file. Apart from that, your EtherBank contract and file are correctly coded for your inheritance structure.

In terms of the rest of your EtherBank code, the comments in my feedback for your Transfer Assignment apply.


Just one additional point…

As you know, when a derived contract inherits functionality from one or more parent contracts, we only need to deploy the derived contract in order to have the inherited functionality available. But if the parent contract(s) are in separate files, then we need to import them into the derived contract’s file.

The situation is different when calling an external function. Unlike an inherited contract, the external contract also needs to be deployed in order to be able to call its functions from another contract. The external function call can be made on an instance of the external contract (based on an interface and the address of the external contract). Because of this, the external contract’s file does not need to be imported. We would only need to import the interface (not the external contract) if this was located in a separate file. So you can also remove…

import "./Government.sol";

Correct :ok_hand:
However, you were using msg.sender and not an address literal. But msg.sender underwent the same change from v0.8 as address literals. You can see this in the 9th bullet point of the New Restrictions section in the Solidity v0.8.0 Breaking Changes you linked to and quoted from:
https://docs.soliditylang.org/en/v0.8.4/080-breaking-changes.html#new-restrictions

e.g.

v0.7.0

pragma solidity 0.7.0;

contract PayableTest {
    /* address literal is payable by default */
    address payable a = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4; //compiles
    /* and can be implicitly converted to a non-payable address*/
    address b = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4; //compiles

    /* msg.sender is payable by default */
    address payable c = msg.sender; //compiles
    /* and can be implicitly converted to a non-payable address*/
    address d = msg.sender; //compiles
    
   function withdraw(uint amount) external {
      /* msg.sender is payable by default */
      msg.sender.transfer(amount); // compiles

      /* address literal is payable by default */
      0x5B38Da6a701c568545dCfcB03FcB875f56beddC4.transfer(amount); //compiles
   }
}

v0.8.0

pragma solidity 0.8.0;

contract PayableTest {
   /* address literal is non-payable by default */
   address payable a = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4; //ERROR
   address b = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4; //compiles
   /* can be explicitly converted to a payable address */
   address payable x = payable(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4); //compiles
   
   /* msg.sender is non-payable by default */
   address payable c = msg.sender; //ERROR
   address d = msg.sender; //compiles
   /* can be explicitly converted to a payable address*/
   address payable y = payable(msg.sender); //compiles

   function withdraw(uint amount) external {
      /* msg.sender is non-payable by default */
      msg.sender.transfer(amount); //ERROR
      /* can be explicitly converted to a payable address */
      payable(msg.sender).transfer(amount); //compiles
      y.transfer(amount); //compiles

      /* address literal is non-payable by default */
      0x5B38Da6a701c568545dCfcB03FcB875f56beddC4.transfer(amount); //ERROR
      /* can be explicitly converted to a payable address */
      payable(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4).transfer(amount);
      x.transfer(amount); //compiles
   }
}
Destroyable.sol
pragma solidity 0.7.5;

import './Ownable.sol';

contract Destroyable is Ownable {
    
    function byeBye() public onlyOwner {
        selfdestruct(owner);
    }
}
Ownable.sol
pragma solidity 0.7.5;

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


import "./Destroyable.sol";

contract Bank is 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);
        msg.sender.transfer(amount);
        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);
        uint previousSenderBalance = balance[msg.sender];
        balance[recipient] += amount;
        previousSenderBalance -= amount;
        
    }
}
contract Destroyable {
    address owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _; //run the function
    }
    
    constructor(){
        owner = msg.sender;
    }
    
    function close() public onlyOwner {
        selfdestruct(owner);
    }
}

Hi Donnie,

Sorry for the delay in replying.

Nearly there now! :raised_hands:

You are inheriting Ownable and Destroyable, but only importing Ownable. Remember…

Can you see what you need to add?

Your Destroyable is looking good now, and it’s nice to see you’ve got the other options for calling selfdestruct() in comments :ok_hand:
Just a couple of points…

It will still execute, but close() does not need to be payable with any of the options, because the function itself doesn’t need to receive any ether. Only our owner address needs to be payable because it’s receiving ether (the remaining contract balance).

You don’t need this line for your Option 2…

The other option would be simply…

selfdestruct(msg.sender);

I explained why this works, here…

And remember… when you run your options, the owner variable doesn’t need to be defined as payable in Ownable — only when you use  selfdestruct(owner);

Once you resolve the error in HelloWorld…

Let me know when I can crack open the champagne! :smiley:

Hi @kopino4,

If you are using v0.8 instead of v0.7.5, then this is correct…

From v.0.8, msg.sender is non-payable by default, which is why you need to explicitly convert it to payable when assigning it to your payable address variable owner in the constructor. Prior to v.0.8 msg.sender was payable by default, and so this explicit conversion wasn’t necessary.

If you only need your owner address to be payable for the purposes of selfdestruct(), then another option is to leave the address variable non-payable, and then explicitly convert owner when passing it to selfdestruct()…

selfdestruct(payable(owner));

There isn’t really a “standard way”, as each situation is different and it’s up to us as developers to work within the confines of the language’s defined syntax rules and come up with the most appropriate solution. The syntax is standardised though, and that is what you need to learn. For example, what I’ve explained to you above about msg.sender in v.0.8 can be found in the Solidity documentation, here (9th bullet point in the New Restrictions section of Solidity v.0.8.0 Breaking Changes):
https://docs.soliditylang.org/en/v0.8.4/080-breaking-changes.html#new-restrictions.

Your destroy() function is looking good, but why have you given it internal visibility? Can we see your complete solution in order to see how you have coded your inheritance structure?
Just to make the code more interesting in terms of the inheritance, how would your solution look if you separated your ownership functionality and contract destruction functionality into 2 separate files — giving you 3 files in total?

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

contract HelloWorld is Ownable, Destroyable {
    

I thought you said that this was wrong in an earlier post. Maybe I misunderstood.

Now, the only error that I get is after the deployment. it states. “currently you have no contract instances to interact with.”

I also changed this:

import "./Ownable.sol";

pragma solidity 0.5.12;

contract Destroyable is Ownable {

    function close() public onlyOwner{
         selfdestruct(owner);
     //option 1
     //address payable receiver = address(uint160(owner));
     //  selfdestruct(receiver);
    // option2
   // function close() public onlyOwner {
  //  selfdestruct(address(uint160(owner)));
  //option next: 
  //function()close public onlyOwner{
    //selfdestruct(msg.sender);  
  //}
       
    }

}

    

it seems to automatically default to “Destroyable”. even though I told it to deploy helloWorld.
because I don’t know why!

1 Like

pragma solidity 0.7.5;

contract Destroyable{

address payable private owner;

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

constructor(){
    owner=msg.sender;
}

function close() internal onlyOwner { //Internal to allow calls only from derived contracts
    selfdestruct(owner);  
}

}

1 Like

pragma solidity 0.7.5;

Contract Destroyable {

address public owner ;

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

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

Hi @StanleyGM,

A few problems here…

  1. You are importing the file with Ownable, but Destroyable isn’t inheriting it. You need to add the relevant code to the contract header.

  2. The aim of this assignment is for our Bank contract to inherit the selfdestruct() and Ownable functionality, so that both are available when we just deploy Bank i.e. we should be able to deploy Bank, perform some transactions, then destroy the Bank contract, transferring any remaining ether balance to the caller of the selfdestruct function. But by marking your byeBye function private, it (and therefore the selfdestruct function it contains) will not be inherited by Bank. Functions in the parent contract with public or internal visibility are inherited, but not those marked private or external.

  3. When the selfdestruct() function is called it will automatically transfer the ether balance remaining in the contract to the address it is called with (in your case, owner ). For this to happen, this address needs to be a payable address. Because you haven’t converted it to a payable address in Destroyable, we need to see your Ownable contract to check that you have defined it as payable there.

  4. 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.

If you get stuck trying to make these modifications on your own, then you can always have a look at other students’ solutions, and the feedback and suggestions they’ve received, posted in this discussion topic. This is where you can also post any questions you have about the assignment, and ask for help, before you post your final solution.

Let us know if anything is unclear, or if you have any questions :slight_smile:

Hi @Lehigr2,

The aim of this assignment is for our Bank contract to inherit the selfdestruct() and Ownable functionality, so that both are available when we just deploy Bank i.e. we should be able to deploy Bank, perform some transactions, then destroy the Bank contract, transferring any remaining ether balance to the caller of the selfdestruct function. When the selfdestruct() function is called it will automatically transfer the ether balance remaining in the contract to the address it is called with. For this to happen, the address (in your case, owner ) needs to be a payable address. Yours is currently non-payable, and the compiler should have indicated this to you by highlighting the line number throwing the error in red. The error message will also tell you what the problem is. If you don’t know, or can’t find out, how to convert owner to a payable address — there are several different ways — then have a look at other students’ solutions, and the feedback and suggestions they’ve received, posted in this discussion topic. This is also where you can post any questions you have about the assignment, and ask for help, before you post your final solution.

To complete each coding assignment, and before moving on to the next lesson, you need to make sure (i) your code compiles without any errors (ii) your contract (in this case the child contract Bank) deploys successfully; and (iii) your deployed contract is able to perform all of the necessary transactions i.e. it does what you expect it to.

As Bank is the derived contract, and therefore the contract we are deploying, 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.

Just to make the code more interesting in terms of the inheritance, you can also try separating your ownership functionality and contract destruction functionality into 2 separate files, giving you a total of 3 files (instead of just 2) in your inheritance structure.

Let us know if anything is unclear, or if you have any questions :slight_smile:

Hi Donnie,

This is now correct, so yes, you must have misunderstood…

Here, I was telling you that, earlier, you had correctly imported Destroyable.sol into helloWorld.sol, but then you removed it.

The changes you’ve made to Destroyable are also correct :ok_hand:

Where are you seeing this error message? This will always be displayed in orange under Deployed Contracts in the Deploy & Run Transactions panel before you have clicked on the orange Deploy button. Before clicking on this button you need to make sure that:

  • all 3 contracts compile without any errors;
  • helloWorld.sol is open on your screen;
  • you have selected HelloWorld from the dropdown menu in the Contract field (just above the Deploy button).

I have copies of the exact same contracts you have posted, and have made the same changes I’ve told you to make and checked that you’ve made them… and my HelloWorld deploys no problem.

Are you importing the exact same file names in terms of exact spelling, capital/lower case letters etc.? e.g. if the Destroyable file is destroy.sol then you need to import "./destroy.sol" and not "./Destroyable.sol"

Another problem could be that you haven’t got all 3 files within the same folder. The file paths…

"./Destroyable.sol"
// or
"Destroyable.sol"

… will only locate the file if it is in the same folder as the file you are importing it into. You can check this in Remix in the workspace you’re using in the File Explorers tab (at the top of the menu). If you have created any workspace sub-folders, and you haven’t got all 3 .sol files in the same folder, then you need to ensure that they are. I haven’t found a way to move files in Remix between folders, so you would need to create a new file in the correct folder, then cut and paste your code into the new file. Make sure you delete the old file, and that your new file has the same name as the old one.

If none of the above solve the problem, then post all of your code in all 3 files, and tell us exactly what names you have saved each file as.

Hi @decrypter,

This is looking promising, but 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. This is a key part of your solution, because the aim of this assignment is for our Bank contract to inherit both the contract destruction and ownership functionalities, so that both are available when we just deploy Bank i.e. we should be able to deploy Bank, perform some transactions, then destroy the Bank contract, transferring any remaining ether balance to the caller of the selfdestruct function (here, the contract owner).

In addition, for the owner to destroy Bank, the owner address needs to be able to call close() to trigger selfdestruct(). This requires a call from an external service such as Remix. By giving close() internal visibility, you are right that it will be inherited by the derived contract, but without seeing your derived contract (which I assume is Bank) there is no way for us to know how you have ensured that close() can be called by the owner. In actual fact, by using internal you overcomplicate the situation, and create a need for additional code to be added to the derived contract. Instead, by making close() public, it would still be inherited by the derived contract and also directly available to external services, without the need to add any supporting code to facilitate this.

Well done for making sure your owner address is payable , so that selfdestruct() is called with a payable address and, therefore, any remaining ether in the contract is paid/transferred to the owner when selfdestruct() is triggered.

Just to make the code more interesting in terms of the inheritance, how would your solution look if you separated your contract ownership functionality and contract destruction functionality into 2 separate files — giving you 3 files in total?

Let us know if anything is unclear, or if you have any questions :slight_smile:

pragma solidity 0.8.4;

import "./ownable.sol";

contract Destroyable is Ownable {
    function close() public onlyOwner {
        selfdestruct(payable(owner));
    }
}
1 Like

Destruct.sol

contract Destruct {

address payable public owner;

function DestroyContract() internal {
    selfdestruct(owner);
}

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

Child file


import "./Destruct.sol";

contract Bank is Destruct {

mapping (address => uint) balances;

function deposit() payable public {
    balances[msg.sender] += msg.value;
}

function getbalance() public returns(uint) {
    return(balances[msg.sender]);
}
function destroy() public {
    require(msg.sender == owner);
    DestroyContract();
}

}

1 Like