Inheritance Assignment

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

[jon_m]

Hi @iceman007!

Your solution is correct, but it involves making an additional change to contract Ownable. Do you know what it is and why?

In addition, have look at @firepol’s post just above, and my reply, and you will see that we can also make our inheritance code in HelloWorld more concise as well.

[iceman007]

Hi Jon

Yes I needed to make my owner as a payable address.

[DeCryptolorian]

First I added this to Hello World:

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

contract HelloWorld is Ownable, Destroyable{

I wondered if you could add multiple imports into one import command, but it did not seem to take, so I did it separately.

Then I created Destroyable.sol:
import “./Ownable.sol”;
pragma solidity 0.5.12;

contract Destroyable is Ownable{

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

I was having some issues with the self destruct at first, but I worked it out. Defining payable was the key in my case.

[jon_m]

Hi @DeCryptolorian,

Nice analysis of your steps, and thought processes

Check this post out — it explains how you can make an additional adjustment in terms of the inheritance. I think you’ll find it interesting.

[matren]

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

contract Destructable is Ownable {
    function destruct() public onlyOwner{
        selfdestruct(owner);
    }
}

pragma solidity 0.6.8;

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

[jon_m]

Hi @matren,

Nice solution

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

I also noticed that in contract Ownable you didn’t state the visibility of

@matren

address payable owner;

I’m interested to know if that was a deliberate decision? Your code still runs without it, but it would make the contract clearer for other developers if you stated what you intend the visibility to be.

[matren]

Thanks for asking.
You’re right, visibility should always be used to state the intend.

For now i don’t know which visibility to use in solidity.
In general i would use the most restricted visibilty possible. In this case it should be then “internal”.

The default visbility in solidity is “public” where other languages normaly use “private” as default.
There must be a reason why “public” was chosen in solidity as the default.

Some statements about this (please correct me if i am wrong):

1. state variables can only be changed through a function call in the contract itself
   (meaning, it cannot be changed from external contracts or through the API exposure of the contract).

2. state variables are visible in the blockchain, therefore there’s no way to hide their contents

So my question would be:
Is there any reason i should make the owner “internal” instead of defaulting it to “public” ?

If i see it correctly public state variables have a “public getter” but still an “internal setter”.

[Brandon]

In the Onwable contract I explicitly converted the owner address to an address payable, like so:

address payable internal owner; // Limit access.

Next, the Destroyable contract reads as such:

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

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

[Jian_Hao_Wei]

contract ownable{

address payable public owner;

modifier onlyOwner(){
    require(msg.sender == owner);
    _; //Continue execution
}
constructor() public{
    owner = msg.sender;
}   

}

contract destroyable is ownable{

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

}

contract HelloWorld is ownable, destroyable{
…
}

[dAnijboned]

I created a new Destroyable contract (which in fact derives from Ownable because only the owner should be able to destroy the contract):

import "./Ownable.sol";

pragma solidity 0.5.12;

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

Then I made our HelloWorld contract Destroyable (I initially inherited from Ownable and Destroyable but then realized that Destroyable was already Ownable :

contract HelloWorld is Destroyable {
    //...
}

Compiled, deployed and executed the close function:

[vm]from:0x147...c160cto:HelloWorld.close() 0x459...d8f30value:0 weidata:0x43d...726d6logs:0hash:0x178...4dce6
Debug
 status 	0x1 Transaction mined and execution succeed
...

I also needed to make the following change in the type of the ower variable of the Ownable contract:

address public owner;

changed to:

address payable public owner;

That’s because when destroying a contract, all it’s funds will be transferred to the owner’s address.

[Thaddeus19]

pragma solidity 0.5.12;

import “./Ownable.sol”;

contract Destroy is Ownable{

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

}