Inheritance Assignment

[cryptoGerry]

pragma solidity 0.5.12;

contract Destroyable {
    address payable private owner;
    
    constructor() public{
        owner = msg.sender;
    }
    
    modifier onlyOwner(){
        require(msg.sender == owner, "Caller is not owner");
        _;
    }
    
    function destroy() public onlyOwner { 
        selfdestruct(owner);  
    }
}

[jon_m]

Great!
As contract Destroyable inherits contract Ownable , you can also streamline things even more by removing the code from contract HelloWorld that explicity inherits contract Ownable. This is because HelloWorld already inherits from Destroyable , and so now indirectly inherits Ownable via Destroyable .

What I’m trying to say is that in contract HelloWorld you can just have:

import "./Destroyable.sol";
pragma solidity 0.5.12;
 
contract HelloWorld is Destroyable {...}

[jon_m]

Great!

In case you didn’t know, before you type in your code to post it here in the forum, click on the </> icon in the menu at the top of the text editor. You will see 2 sets of 3 back ticks.
```
```
If you now input your code between these, you will end up with nicely formatted code, which is then also easier for you to organise with the correct spacing and indentation etc. You should end up with something like:

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

contract Destroyable is Ownable {

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

}

Hope that helps

[jon_m]

Good start!..but don’t forget the contract declaration and to state the name of the actual contract it is inheriting (not just the file).

Also, would you make any modifications to HelloWorld ?

If you post the finished product I’ll check it for you

[EdsonRamirez]

pragma solidity ^0.6.0;

import “./Ownable.sol”;

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

And also imported Destroyable.sol to helloworld contract and inherited from it along with ownable.

[Caupuri]

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

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

[jon_m]

Hi @cryptoGerry!

Great code!

By the way, what amendments did you make to  contract HelloWorld  in terms of its inheritance of  contract Destroyable ?

Also, if you amend  contract Destroyable  so that it inherits from  contract Ownable , you won’t need to repeat the owner variable, the modifier or the constructor, as these will be inherited from Ownable. However, you would then need to make an amendment to EITHER your destroy function, OR the owner variable in contract Ownable. See if you can work out what it would be and why…

[jon_m]

Great @EdsonRamirez!

By the way, check this post out — it explains how you can make an additional adjustment.

[Yves_T]

Ownable.sol

pragma solidity 0.5.12;

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

Destroyable.sol

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

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

[cryptoGerry]

Thank you.
I did not work with the original HelloWorld contract that was used in the videos.
I just created a generic contract that I could use with other contracts that I had in remix and that didn’t have the Ownable function.
But you’re right, if I wanted to do a real separation of concerns here, the right thing to do would be to have the constructor and modifier within a contract Ownable.sol, and then just the call to selfDestruct () in the contract Destructible.sol.

[mjwatson10]

//Destroyable.sol file

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

contract Destroyable is Ownable{

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

}

//HelloWorld.sol file

import “./Ownable.sol”;
import “./Destroyable.sol”;
pragma solidity 0.5.12;

contract HelloWorld is Ownable, Destroyable{

[mjwatson10]

Hi @filip ,
Is it better that it be written like you did
import “./Ownable.sol”;
pragma solidity 0.5.12;

contract Destroyable is Ownable{

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

}

rather than how i did originally?

[firepol]

Destroyable.sol

import "./Ownable.sol";

contract Destroyable is Ownable {

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

}

HelloWorld.sol

import "./Destroyable.sol";

contract HelloWorld is Destroyable {
[...]

I made Destroyable inherit from Ownable, because to be Destroyable, a contract needs some safety so that only the owner can destroy it. So in the end if you want a contract that is Destroyable, you also get the “Ownable” properties and functions.

I also had to fix some error because “owner” had to be a payable address so:

Ownable.sol

contract Ownable {

    address payable public owner;

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

    constructor() public {
        owner = msg.sender;
    }
    
    function setNewOwner(address payable _address) public onlyOwner returns (address) {
        require(_address != address(0));
        owner = _address;
        return owner;
    }
}

[firepol]

Hi @filip (hello to anybody else too, who wants to answer )

I’ve seen in your solution:

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

contract HelloWorld is Ownable, Destroyable {

in my solution I did simply:

import "./Destroyable.sol";

contract HelloWorld is Destroyable {

In HelloWorld I didn’t import Ownable, because that is imported already in Destroyable. Also I didn’t specify is Ownable, because Destroyable is already ownable.

Why did you do that redundancy? Do you agree that it’s not necessary? I tested my solution and of course it works, so I assume it’s ok

[iceman007]

Destroyable contract which inherets fromOwnable

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

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

And the HelloWorld contract inherets from the Destroyable contract

contract HelloWorld is Ownable, Destroyable{
}

[jon_m]

Hi @Yves_T!

Nice solution

By the way, what amendments did you make to contract HelloWorld in terms of its inheritance?

[jon_m]

Hi @mjwatson10!
You may find this answers your question:

@jon_m

But the solution video left me with one question:
Why is it necessary to assign msg.sender (the owner’s address) to a local variable ( receiver ) with a payable-address type, and then pass receiver to selfdestruct() as its parameter? Can’t this be made more concise and simpler by just passing msg.sender directly?

i.e selfdestruct(msg.sender);

Having looked at some of the other solutions posted, I can see that the more concise selfdestruct(msg.sender); has been widely used. I then found that this point has already been raised and answered in some posts from @Guy and @filip at the beginning of this topic. I’ll link to the discussion here via some quotes, in case others have the same question, but wouldn’t otherwise see the relevant posts:

@Guy

After looking at the solution - isn’t msg.sender is already “address payable”?
why do we need to convert it again to a “payable address receiver”?

@filip

You are correct that msg.sender is already a payable address. But what we do in the code is not a conversion. We are saving msg.sender to a variable. And if we want the address to remain payable, we need to save it to a payable address. If we were to save msg.sender to a non-payable address variable, then it wouldn’t be payable anymore.

@Guy

…but can we avoid the saving of msg.sender as a variable and use:

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

It works…and only the owner can destroy the contract so it looks quite safe.

Are there any issues that I am not aware of involved?

@filip

Yes, that’s absolutely safe. That’s a better solution than to use a variable as a go-between. But sometimes people find it clearer and easier to read when we break the code up into multiple steps.

I think the points made in these posts are really helpful

[jon_m]

Hi @mjwatson10

Check this post out — it explains how you can make an additional adjustment in terms of the inheritance.

[jon_m]

Nice detailed analysis and summary, @firepol!

And I like the idea of the additional setNewOwner Function

@firepol

function setNewOwner(address payable _address) public onlyOwner returns (address) {
        require(_address != address(0));
        owner = _address;
        return owner;
    }

You should remove returns(address) and return owner because this function is a setter not a getter, and these additional pieces of code don’t actually do anything anyway. Contract Ownable automatically generates a getter for owner which is a state variable and has public visibility, and so is inherited by Hello World, and so this enables us to get the owner’s address from there.

Have a go at making these changes and you’ll see what I mean.

[jon_m]

Hi @firepol!

You are exactly right in your observation. There is actually some discussion about this a bit further up in this thread…

@jon_m

As a result of the following comment from @ivga80, I’ve also learnt that if contract Destroyable inherits contract Ownable , then we can remove the code from contract HelloWorld that explicity inherits contract Ownable . This is because HelloWorld already inherits from Destroyable , and so now indirectly inherits Ownable via Destroyable .

@ivga80

In theory, Since you already imported Ownable.sol in contract Destroyable. You don’t need to import Ownable.sol in your BasicSolidity contract to.

I think maybe in the solution code, contract HelloWorld was left inheriting Ownable (as well as Destroyable) from the previous lecture. It isn’t an error, but your inheritance composition is the more efficient, and everyone agrees with that.