Inheritance Assignment

Ethereum Smart Contract Programming 101
#1152

Great job @loso :ok_hand:
… especially as the link no longer worked, and you have completed this assignment based on your own research :muscle:

Apologies for the delay in giving you feedback on this assignment, and also for the inconvenience caused by the link no longer working. It appears as though the website hosting the article has shut down, or something similar has happened. We’ve now found a suitable replacement article, and modified the assignment instructions accordingly. The assignment itself is still the same, but if you would like to read the new article and more comprehensive instructions, this update is now available for you to view on the Academy website.

Feedback on your solution

Well done for realising that the address passed to selfdestruct() needs to be payable, in order for any remaining ether in the contract to be paid/transferred to this address when selfdestruct() is triggered. Your method, calling destroySmartContract() with a payable address parameter, is perfectly correct. An appropriate address to send these remaining funds to would be the contract owner address. You can do this by passing either msg.sender, or owner (inherited from Ownable), to selfdestruct(), which also means destroySmartContract() no longer needs an address parameter.

You can use msg.sender because, as a result of the onlyOwner modifier, it can only ever be the contract owner address. Also, msg.sender is already a payable address by default (only before Solidity v0.8) and so doesn’t need to be explicitly converted.

However, if you pass owner you would need to:

  • either define it as a payable address state variable in Ownable;
  • or convert it locally within the destroySmartContract() function body, either using a local variable before it is passed to selfdestruct(), or convert the actual argument e.g. 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.

@Anvar_Xadja is also right about this…

Let me know if you have any questions about any of these points :slight_smile:

#1153

Nice solution @Anvar_Xadja :ok_hand:

Apologies for the delay in giving you feedback on this assignment, and also for the inconvenience caused by the link no longer working. It appears as though the website hosting the article has shut down, or something similar has happened. We’ve now found a suitable replacement article, and modified the assignment instructions accordingly. The assignment itself is still the same, but if you would like to read the new article and more comprehensive instructions, this update is now available for you to view on the Academy website.

Feedback on your solution

Well done for realising that the address passed to selfdestruct() needs to be payable , in order for any remaining ether in the contract to be transferred to this address when selfdestruct() is triggered. Your method, calling destroyContract() with a payable address parameter, is perfectly correct, and provides the flexibilty for the owner to transfer to any address of their choosing :ok_hand:

If we knew in advance that any remaining funds should only be transferred to the contract owner’s address (which would be an appropriate and secure restriction), then alternative solutions would be to pass either msg.sender , or owner (inherited from Ownable), to selfdestruct(), which also means destroyContract() no longer needs an address parameter. I’ve gone into further detail about how to implement these alternatives in my feedback to @loso (which I’ve also tagged you in).

If the emit statement is placed after the call to selfdestruct(), the event won’t be emitted because the contract has been destroyed once selfdestruct() has executed, and so the emit statement is never reached. You could fix this by placing the emit statement before the selfdestruct() function call, but I don’t think this is wise, because we would be emitting the event before the actual event (contract destruction) has ocurred. On the other hand, if selfdestruct() failed, then the function would revert, including the emitted event, but I would still be reluctant to place the emit statement first. This needs some further investigation. What are your initial thoughts on the position of the emit statement?

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 you have any questions :slight_smile:

Inheritance & External Contracts - Discussion
#1154

Hi Joseph,

Apologies for the late response!

Depending on the inheritance structure you’re using, the onlyOwner modifier is either already defined within your Destroyable contract, or is inherited by Destroyable from Ownable. All modifiers in a base contract are inherited by the derived contract.

Available modifiers are applied to functions by referencing the modifier’s identifier (name) in the function header, as follows:

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

It’s as simple as that :smiley:

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

#1155

Hi @ildevirdee,

Just to let you know, that the new article and updated assignment instructions are now available for you to view on the Academy website.

Once again, apologies for the inconvenience caused by this.

#1156

Hi Douglas,

Just to let you know, that the new article and updated assignment instructions are now available for you to view on the Academy website.

Once again, apologies for the inconvenience caused by this.

#1157

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:

Inheritance & External Contracts - Discussion
#1158

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
#1159

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

1 Like
#1160

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
#1161

@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
#1162

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:

#1163

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
#1164

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
#1165

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
#1166

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. 
    }
}
#1167

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
#1168 
//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
#1169

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:

#1170

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:

#1171

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