Inheritance Assignment

Hi @raphbaph,

Good to hear :slight_smile:

You raise an interesting point. I’ve been discussing the issues related to emitting an event for selfdestruct() with @Anvar_Xadja …

Apart from the above points, it’s also worth bearing in mind that emitting an event doesn’t necessarily mean that users will see that infomation, and so doesn’t actually prevent them from subsequently calling the destroyed contract’s functions, incurring a gas cost, and losing any ether sent. There would still need to be some form of communication to all users informing them of the contract’s destruction, and what the consequences and implications of this are in terms of what they must and mustn’t do from now on. This is also why, in practice, other types of security mechanisms, such as proxy contracts, are implemented to deal with these kinds of emergency situations. You will learn more about the practicalities of this if you go on to take the Smart Contract Security course.

Let me know if you have any questions :slight_smile:

Ownable Contract

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

Destroyable Contract

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {

  function destroy() public onlyOwner {
   selfdestruct(msg.sender);
  }
}

Bank Contract’s beginning

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

contract Bank is Ownable, Destroyable {
1 Like

Definitely would emit before self destruct. But interesting point about not being able to verify if contract then actually self destructed…

1 Like

This is the Ownable contract with the file name of InheritancePart2Ownable.sol

pragma solidity 0.7.5;

contract Ownable{
     mapping(address => uint)balance;
    
    address owner;
    
    event depositCompleted(uint amount, address indexed depositedTo);
    event transferCompleted(address indexed sentBy, uint amount, address indexed receivedBy);
    event withdrawalCompleted(address indexed withdrawTo, uint amount);
    event selfDestruction(address indexed balancePostDestroyedSentTo);
    event selfDestructionv2(address indexed balancePostDestroyedSentTo);
    
    modifier onlyOwner{
        require(msg.sender == owner);
        _; // run the function
    }
    
    constructor() {
        owner = msg.sender;
    }
    
}

This is the Destroyable contract with the file name of InheritancePart3SelfDestruct.sol that inherits Ownable

pragma solidity 0.7.5;

import "./InheritancePart2Ownable.sol";

contract Destroyable is Ownable {
    function selfDestruct() public onlyOwner payable {
    emit selfDestruction(msg.sender);
    selfdestruct(msg.sender);
    
    //test if emit will still run after selfdestruct invoked (it doesn't)
    emit selfDestructionv2(msg.sender);
    //Hypothesis: anything after selfdestruct doesn't run as the smart contract is effectively destroyed
    //But the showBalance still work as stated in https://betterprogramming.pub/solidity-what-happens-with-selfdestruct-f337fcaa58a7
}
}

For the Bank contract (just for the import and contract header mainly)

import "./InheritancePart3SelfDestruct.sol";

contract Bank is Destroyable{
   
    function deposit() public payable{
        balance[msg.sender]+=msg.value;
        emit depositCompleted(msg.value, msg.sender);
        
    }

Ive structured the inheritance whereby Ownable contract is the parent to Destroyable and Bank inherits from Destroyable somewhat making the relationship of the contracts to be Ownable>Destroyable>Bank which probably defines the relationship as a multi-level inheritance?

*Another question would be:

contract Bank is Destroyable, Ownable 

results in this error:
“InheritancePart1Bank.sol:7:1: TypeError: Linearization of inheritance graph impossible contract Bank is Destroyable, Ownable{ ^ (Relevant source part starts here and spans across multiple lines).”

whereas if:

contract Bank is Ownable, Destroyable

There isn’t a warning. Does that mean that the sequence of contract inheritance in the contract header is important to how the contract runs?

1 Like

@jon_m, sorry for not getting back to you sooner. I have been struggling to gain insight into other courses before my subscription is up. I will be taking a few days off from the course and in the meantime, I will be making progress elsewhere but I aim to return in a short while. Thank you very much for your time.

1 Like

No problem @JosephGarza, I totally understand — no need to apologise. Just pick things up with this course again when you’re ready. A short break, and focussing on something else can be just as beneficial as ploughing on when you get to that saturation point, which can happen at different stages with different courses. The last thing you need is to feel overly pressurised. Take care, and see you soon :slight_smile:

I don’t think the problem is that a user can’t verify if the contract has been destroyed or not, but rather that they would need to do this pro-actively. They wouldn’t be automatically prevented from calling one of the destroyed contract’s functions. The contract owner could notify users that the contract has been destroyed, via an alert in the frontend interface. This could perhaps be triggered by an event listener which listens for an emitted event, if it is decided to emit an event before selfdestruct() is actually called.

1 Like

Hi @jon_m how are you ? Hope you’re doing well.

I think I’ve figured it out, works in Remix so far:

Bank.sol

pragma solidity 0.7.5;

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


contract Bank is Ownable, Destroyable {
    
    mapping(address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    function deposit() public payable returns (uint)  {
        balance[msg.sender] += msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function withdraw(uint amount) public onlyOwner returns (uint){
        require(balance[msg.sender] >= amount);
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        return balance[msg.sender];
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }
    
    function getOwner() public view returns (address) {
        return owner;
    }
    
    function transfer(address recipient, uint amount) public {
        require(balance[msg.sender] >= amount, "Balance not sufficient");
        require(msg.sender != recipient, "Don't transfer money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
    
}

Ownable.sol

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

Destroyable.sol

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

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

I’ve been playing around in Remix a little which answered my questions regarding the selfdestruct function.

For a while I was trying to have Destroyable.sol below Ownable.sol, but it didn’t work because of the onlyOwner modifier. I looked up to comma (contract Bank is Ownable, Destroyable), which was what was giving me issues, but it makes sense. If you have a complex inheritance, do you put a lot of commas ?

After some time when you go through the material over and over again and it becomes clearer and everything comes together it’s really cool !

Thank you for your help !

Best wishes, Klemen

1 Like

Hi Jon, i was hoping you could help me correct my understandin of a struct. i wrote out the code and tried to write my understanding for each line. Please let me know if my understanding is correct and if not i would love to get ur insights on it thank you

Structs - similar to object in java ,you can define what properties a person should have

pragma solidity 0.7.5;
contract Classes {
struct Person{ // (Ur defining what properties a person should have in this area)
uint age;
string name;
}
Person[]people; // This is a person array of people this will allow you to keep adding more than one person

function addPerson(uint _age, string memory _name) public {
Person memory newPerson = Person(_age, _name); // ur using the defination of a person here
people.push(newPerson); // Then you push the person into the array here
}
function getPerson( uint _index) public view returns (uint, string memory){ //we return the properties of the person

Person memory personToReturn = people[_index]; // here is were we get the person of the people array/ This is where we link the person to there index number eg: Tom will link to the [0] index, Juile will to the [1] index etc…

return (personToReturn.age, personToReturn.name); // This is how you access the properties of a struct
}
}

1 Like

Hi @jahh,

You’ve explained what your code is doing very well, and you appear to have a good understanding of how structs work in Solidity. Below is your original code and comments, with my own comments added…

pragma solidity 0.7.5;

contract Classes {

    struct Person { // (Ur defining what properties a person should have in this area)
        uint age;
        string name;
    }
    /* CORRECT -- we are effectively defining a new data type of type Person.
       The struct is like a template which, as you say,
       defines the identifier (name) and the data type of each property. */ 
    
    Person[] people; // This is a person array of people this will allow you to keep adding more than one person
    /* CORRECT -- again, the important thing here is that the array is defined with a Person data type.
       So, this array must hold struct instances based on the template defined in the Person struct. */

    function addPerson(uint _age, string memory _name) public {
        
        Person memory newPerson = Person(_age, _name); // ur using the defination of a person here
        // CORRECT -- we are creating a new Person struct instance, based on the template we have defined with the Person struct.
        // If we call addPerson() with the arguments: 25, "Bob" ...
        // ... the new Person struct instance created could be visually represented as {age: 25, name: "Bob"}
        // This new Person struct instance is stored temporarily in a local variable (newPerson) ...
        //  ... so we need to define this local variable with the Person data type.
        
        people.push(newPerson); // Then you push the person into the array here
                                // CORRECT
    }
    
    function getPerson(uint _index) public view returns (uint, string memory){ // we return the properties of the person
                                                                               // CORRECT

        Person memory personToReturn = people[_index]; // here is were we get the person of the people array
        // This is where we link the person to there index number eg: Tom will link to the [0] index, Juile will to the [1] index etc…
        // CORRECT -- we reference a specific Person instance in the people array ...
        // ... using the index number corresponding to their position within the array.
        // The specific Person instance we have referenced is assigned to, and stored temporarily in, a local variable (personToReturn) ...
        // ... so we need to define this local variable with the Person data type.
                   
        return (personToReturn.age, personToReturn.name); // This is how you access the properties of a struct
        // CORRECT -- we access (and then return) the individual property values of the Person instance using dot notation. 
    }
}

Thank again Jon. Your a great teacher!
Btw the Person data type, most it always be written as - Person[] people? could we just write it as Person[] ?
And out of curiosity, under the function addPerson is there another way to write the code
Person memory newPerson = Person(_age, _name)?
The reason i ask, is i tried different things to see if it would work like. Person memory = Person(_age, _name) etc…
of course it didn’t work , problem is i didn’t really know why it didn’t work :frowning:

1 Like
//This is Destroyable.sol
pragma solidity 0.7.5;
import "./Ownable.sol";

contract Destroyable is Ownable {
    
    function destroy() public onlyOwner {
        address payable receiver = msg.sender;//Set the reciever of the remaining ether at self destruct
        selfdestruct(receiver);
    }
}
// This is HelloWorld.sol
pragma solidity 0.7.5;
import './Ownable.sol';
import './Destroyable.sol';

contract HelloWorld is Ownable, Destroyable {
//The rest of the code
}
1 Like

Hi @ibn5x,

This is an excellent attempt at implementing a slightly more complex selfdestruct() functionality :ok_hand:
Your code is correct and when your Banking* contract is deployed it will successfully inherit Ownable.
( * better to start all contract names with a capital letter, for consistency)


Here are some suggestions for improvements in terms of the overall structure, and for conciseness:

(1) Your _stash address is associated with the selfdestruct() functionality, which could be considered separate to both Banking and Ownable. You could create a separate contract Destroyable for the closeBank function and the stash address state variable. This would give you a 3-contract multi-level inheritance structure. It also gives the flexibility to reuse the selfdestruct() functionality and/or the contract-ownership functionality in other derived contracts, if needed, and without having to duplicate any code.

(2) As the _stash address is only used to receive any remaining ether in the contract on its destruction, you could define this from the outset as a payable address state variable. This means you don’t have to convert it in closeBank()

(3) To avoid having to hard code the _stash address within the smart contract, you could add a constructor with an address parameter to Destroyable, which allows the contract owner to set a specific _stash address on deployment of Banking. If we have a constructor with a parameter in a base contract, then we need to specify this in our derived contract. One way to do that is as follows:

// Base contract (example with a uint state variable instead of an address)
uint myNumber;
constructor(uint _num) {
    myNumber = _num;
}

// Derived contract (the contract being deployed)
contract myDerivedContract is myBaseContract {
    constructor(uint _num) myBaseContract(_num) {}
}

Here is the documentation for base constructors, in case you are interested in reading more detail about this:

https://docs.soliditylang.org/en/v0.8.1/contracts.html#arguments-for-base-constructors

(4) In the closeBank() function, it is only the address passed as an argument to selfdestruct() which needs to be payable , and not the closeBank() function itself. A function only needs to be marked payable when it needs to receive (not send) ether. That’s why we marked our deposit function in Bank as payable, but not our withdraw function. Marking closeBank() as payable doesn’t prevent your code from compiling, or the function from executing, but it is bad practice and poor risk management not to restrict a function to non-payable if it doesn’t need to receive ether, because if someone sends ether by mistake, this amount will still be transferred to the contract. In this particular instance, you may well think that this doesn’t create too much of a risk — and you would be right. Only the contract owner can call closeBank(), and even if they were to send ether when calling the function, by mistake, essentially this would be transferred to the contract and then immediately transferred to the _stash address together with the rest of the remaining funds. One would assume that the contract owner also has control over the _stash address, or could easily arrange for a refund of the ether transferred there by mistake. However, the principle that this kind of mistake should be prevented from happening in all situations, still stands. As we are programming money, we need our code to be as risk-averse as possible.

I hope you find these extra considerations interesting. How about trying to implement them?

Let me know if you have any questions :slight_smile:

Hi @RBeato,

Firstly, apologies for the delay in giving you some feedback on your solution for this assignment.

Thanks for also alerting us to the issue with the linked article. It’s seems as though the website hosting the article shut down, or something similar happened. We’ve found a suitable replacement article, and modified the assignment instructions accordingly. The new article and updated assignment instructions are now available for you to view on the Academy website.

You’ve actually produced a very good solution yourself without the article :muscle:

Well done for realising that the address passed to selfdestruct() needs to be payable, so that any remaining ether in the contract can be paid/transferred to that address when selfdestruct() is triggered.

There are other ways to code this (for example, passing the contract owner’s address to the selfdestruct function). If you don’t want to repeat the assignment for the new article and instructions, then you could just take a look at some of the other students’ solutions, and the feedback and comments they’ve received, posted here in this discussion thread, to get an idea of the alternatives.

The only thing that’s missing from your code is an import statement in your SelfDestruct.sol file, to import Ownable.sol.

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.

Once again, apologies for the inconvenience caused by the missing article. Let me know if you have any questions :slight_smile:

Great solution @TolgaKmbl :ok_hand:

Apologies for the delay in giving you some feedback for this assignment.

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 :slight_smile:

1 Like

Right @jon_m , that one slipped beneath my radar.

Thank you for your feedback and have a good day. :+1:

1 Like

Hi @KK_Cheah,

Apologies for the delay in giving you feedback for this assignment.

On the whole, your code to implement selfdestruct() and the inheritance structure is good :ok_hand: But I wouldn’t place the event declarations and the mapping in Ownable, because these are specific to the derived contracts (SelfDestruction event to Destroyable, the 3 transaction events and the mapping to Bank). Your solution will work when you deploy Bank, but the point of placing specific functionality into separate contracts is to create modular and reusable code, which can be inherited by other derived contracts as well. The derived contracts will differ, and so the wider the range of functionality we include in the base contracts, the less reusable they will be.


Yes, you have implemented a multi-level inheritance structure, and this is indeed the best approach, because it is more streamlined :ok_hand: Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.


Yes, that’s correct. Because Ownable is the parent of Destroyable, if both contracts are explicitly inherited by Bank, then we need to declare them in order of hierarchy. That’s what I think is meant by “linearization”.
Here is a great discussion topic about this very point, which poses an interesting inheritance puzzle (more complicated than just our 3-contract multi-level inheritance), and the answer is very well explained.

https://ethereum.stackexchange.com/questions/63564/question-regarding-linearization-of-inheritance


Your hypothesis is correct :+1:
The functions of the destroyed contract can still be called (including showBalance), which is a major disadvantage with selfdestruct(), and the impact of this needs to be carefully considered. For example, if a user doesn’t know the contract has been destroyed and sends ether to the contract, it will be lost!


In selfDestruct(), it is only the address passed as an argument to selfdestruct() which needs to be payable , and not the selfDestruct() function itself. A function only needs to be marked payable when it needs to receive (not send ) ether. That’s why we marked our deposit function in Bank as payable, but not our withdraw function. Marking selfDestruct() as payable doesn’t prevent your code from compiling, or the function from executing, but it is bad practice and poor risk management not to restrict a function to non-payable if it doesn’t need to receive ether, because if someone sends ether by mistake, this amount will still be transferred to the contract. In this particular instance, you may well think that this doesn’t actually matter — and you would be right. Only the contract owner can call selfDestruct(), and even if they were to send ether when calling the function, by mistake, essentially this would be transferred to the contract and then immediately transferred back to them, together with the rest of the remaining funds. However, in other situations, marking functions as payable, when they don’t need to be, can have negative consequences, and so we should get into the habit of not doing this in all cases.


Finally, I would avoid naming the function which contains selfdestruct() “selfDestruct” as well. Even though the 2 identifiers are different by 1 capital letter, this could still cause confusion when reading or manipulating your code.

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

Hey Klemen,

Sorry for the delay in getting back to you on this.

This is now really good :muscle:
Well done for persevering! …and it’s true what you say…

I’m glad I’ve been able to help you get there :smiley:


You could have the Destroyable contract below the Ownable contract in the same file, and onlyOwner won’t cause an issue. Don’t get confused between contracts and files. You could do the following:

// myContracts.sol
pragma solidity 0.7.5;

contract Ownable {
    address payable owner;   /* Notice I can restrict the visibility
                                to internal (see explanation below) */
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    constructor() {
        owner = msg.sender;
    }
}

// These 2 lines NOT INCLUDED
// pragma solidity 0.7.5;    // compiler version only declared once per file
// import "./Ownable.sol";   // Ownable is already in the same file 
contract Destroyable is Ownable { 
    function close() public onlyOwner { 
        selfdestruct(owner);  
    }
}

If Bank explicitly inherits 2 contracts (whether they are in separate files or not), you need to add them both to the Bank contract header, separated by a comma, and in the order they appear in the inheritance hierarchy (“most base” to “most derived”). If you have Ownable and Destroyable in the same file, then the only difference will be that you only import 1 file, instead of 2 e.g.

pragma solidity 0.7.5;
import "./myContracts.sol";

contract Bank is Ownable, Destroyable { ... }

Multiple inheritance is where the same, single derived contract inherits more than one (multiple) base contract. If the derived contract inherits more than 2 base contracts then, yes, you will have 2 or more commas e.g.

// Multiple inheritance structure
contract A { ... }
contract B { ... }
contract C { ... }
contract D is A, B, C { ... }

However, where we have multiple inheritance within a multi-level inheritance structure, the “most derived” contract does not need to explicity inherit the “most base” contract, because inheritance occurs implicitly via the “mid-level” contract(s) e.g.

contract A { ... }

contract B is A { ... }
contract C is A { ... }

contract D is B, C { ... }   /*or*/   contract D is C, B { ... }
// either order is possible because B & C are at the same level in the hierarchy

You can probably now see that you can also streamline the inheritance structure of your 3 contracts, by using multi-level inheritance. If Destroyable inherits Ownable, then Bank only needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable i.e.

contract Ownable { ... }
contract Destroyable is Ownable { ... }
contract Bank is Destroyable { ... }

Just one other observation…
Because you have added a public getter (getOwner) to Bank, to retrieve the contract owner’s address, owner no longer needs to be public. You can give it internal visibility instead, which you don’t have to explicitly define, because state variables have internal visibility by default.


Just let me know if you have any further questions :slight_smile:

1 Like

Hey @jahh,

I’m glad you find my explanations helpful :smiley:

If you’re defining an array state variable, you need to give it a name (an identifier) otherwise it can’t be referenced. In Solidity there are 2 pieces of information that we must always have to define a variable:

  • data type
  • identifier (name)
// e.g. we can't define a variable as just
uint;  // => ERROR
// but we can have
uint myNumber;

With an array, we need to define what data type the array will contain. All of the elements in the array must have the same data type

// e.g
uint[] myArray;  // this array will contain unsigned integers

/* If we want our array to contain values which are struct instances,
   we define that with the struct name e.g. */
struct Country {
    string name;
    string capitalCity;
    uint populationMillions;
    bool visited;
}
// But you still have to give your variable an identifier (a name)
Country[] myArray;
// You can't just define it as...
Country[]; // => ERROR

So, because your struct is called Person you need to define your array of Person instances with data type Person[] and also give it a name:

Person[] people;
// but you don't have to call it people e.g.
Person[] employees;

If you want your variable to only hold a single employee, based on the Person struct, then you would define it as follows:

Person employee;
/* the single employee this variable can hold
   could be visually represented as follows */   {name: Bob, age: 25}

If it’s not a state variable (e.g. a local variable within a function) then for elementary data types the same syntax rules apply e.g.

uint num;

function multiplyByTen() public {
    uint myLocalNumber = num * 10 ;
    num = myLocalNumber;
}

If our local variable needs to store a complex data type (string, array or struct instance) we need to declare the data location as well as the data type and the identifier (name). Again, if we don’t give it an identifier, we can’t then reference it to either assign a value to it, or to access its value e.g.

// assumes the same Country struct from the example above
Country[] myArray;

function addCountry(
    string memory _name,
    string memory _capital,
    uint _pop,
    bool _visited
) public {
    Country memory newCountry = Country(_name, _capital, _pop, _visited);
    myArray.push(newCountry);
}

An alternative to this code is:

Country memory newCountry;

newCountry.name = _name;
newCountry.capitalCity = _capital;
newCountry.populationMillions = _pop;
newCountry.visited = _visited;
    
myArray.push(newCountry);

One of the place you will see (or can use) the data type without the identifier is when we define the data type of the values returned from a function, in returns() e.g.

function getCountryInfo(uint _index) public view
returns(string memory, string memory, uint, bool) {
    return(
        myArray[_index].name,
        myArray[_index].capitalCity,
        myArray[_index].populationMillions,
        myArray[_index].visited
    );
}

If you want to return the whole struct, or even the whole array of structs, then you could have:
(Remember, before v0.8 you need to activate the ABI coder v2 to be able to do this)

// retrieves one of the country instances from the array
function getCountry(uint _index) public view returns(Country memory) {
    return(myArray[_index]);
}

// retrieves the whole array of country instances
function getMyCountries() public view returns(Country[] memory) {
    return(myArray);
}

Note, that you could also have given the return values an identifier, but in the examples above this is optional.


Hopefully, this gives you the explanations you need :sweat_smile:

Just let me know if anything is still unclear, or if you have any further questions :slight_smile:

This is my code:

Ownable.sol

contract Ownable {

 address payable owner;


constructor(){
     owner = msg.sender; 
}

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

}

Destroyable.sol

import “./Ownable.sol”;

contract Destroyable is Ownable {

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

}

Bank Contract (HelloWorld.sol):

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

contract Bank is Ownable, Destroyable { … }

It all works awesome :slight_smile:

I understand I’ve done it a little different to the solution, I’ve made the owner address attribute payable and used that as the payable address if the contract selfDestructs - which I think makes sense as you wouldn’t want anyone other than the owner to recieve the ether right?

Also Bank inherits from Owner and Destroyable, and Destroyable inherits from Owner, so I guess Bank doesn’t need to inherit directly from Owner either right? It works when I remove ‘is Owner’ from bank anyway

Thanks
Ismail

1 Like