Inheritance Assignment

[thecil]

Already solved by private message .

Carlos Z.

[MrSeven]

Here is my Destroyable contract code:

pragma solidity 0.7.5;

contract Destroyable {

address owner;

constructor()   {
    owner = msg.sender;
}

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

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

}

[Depth-Hoar]

Here is my Destroyable contract that can be called upon.

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

contract Destroyable is Ownable {

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

[saf]

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

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

[DUUKA_SMASH]

Yay…Destroy function worked. Finally! now i can actually put the variable “msg.sender” in the body of my function because it just would not let me execute before. Thanks man!

[madingo]

Inheritance Assignment.
Destroyable Contract:

pragma solidity >0.6.0 ;

contract Storage {
address payable private owner ;
uint256 number ;

constructor ( ) {
owner = msg.sender ;
}

function store(uint256 num) public {
num = num ;
}

function retrieve( ) public view returns ( uint256 ) {
return number ;
}

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

[Jethrodog]

My code for the Inheritance assignment:

pragma solidity 0.7.5;

contract Destroyable {

address payable internal owner;

constructor(){                    
     owner = msg.sender;  
}

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


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

}

[Jethrodog]

In my code, I put the address owner as payable & internal.
Do I have to have the internal stated? What if I don’t put anything?

Ben

[jon_m]

Nice Destroyable contract, @saf

But don’t forget the all important line  pragma solidity 0.7.5;

What changes did you also have to make to the Bank contract in terms of inheritance?

[jon_m]

Hi @Jethrodog,

That’s a well thought out solution

@Jethrodog

In my code, I put the address owner as payable & internal.

That’s correct to define the owner state variable as payable, because you have used that variable as the argument in the selfdestruct function, and not msg.sender . msg.sender is payable by default, so if you call selfdestruct with that instead, then owner doesn’t need to be payable. It’s perfectly fine to use either as the argument. It has to be a payable address because this is the address any remaining contract balance will be paid to if the contract is destroyed.

@Jethrodog

Do I have to have the internal stated? What if I don’t put anything?

You don’t have to specify internal visibility with the keyword, because state variables have internal visibility by default (if you don’t state any visibility). However, to avoid confusion when reading the code, you can also decide to specify this with the keyword.

You can also make your code more modular by separating out the “destroy contract” functionality into a separate contract. You can then structure everything using multi-level inheritance:
Bank inherits Destroyable,
Destroyable inherits Ownable.
(Bank therefore inherits Ownable indirectly via Destroyable, because Destroyable already inherits Ownable).

At the moment, because you have combined both Ownable and Destroyable in one contract, if you don’t need to make owner available in the Bank contract, then you could also give owner private visibility. However, if you use Ownable and Destroyable as 2 separate contracts as I’ve suggested above, then owner will need internal visibility (stated or by default), because it will need to be inherited by Destroyable so it can be used in the selfdestruct function.

Also, can we see how you’ve coded the start of your Bank contract to include the inheritance?

[Jethrodog]

Thanks Jon.
I was having troubles with the inheritance of the Ownable contract when I had it separate, not sure what it was, but I didn’t spend much time trying to figure out why, I just decided to make it easier and put it into one.
I looked at Filip’s solution later, where he has them separated, and saw that it works that way.
I noticed he had an extra line of code in his solution, where he has to create and set an address variable called receiver as payable and then initialize it as msg.sender, then use it in the selfdestruct call as the argument.
Is there any reason why he wouldn’t just put msg.sender in there and skip this step?

Here is the start of my Bank contract showing it inheriting the Destroyable:

pragma solidity 0.7.5;

import “./self_destruct.sol”;

contract Bank is Destroyable { …

Thanks!
Ben

[Samuel_Carmona]

destructible.sol:

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

contract Destructible is Ownable {
    
    function destroy() public onlyOwner {
        selfdestruct(msg.sender);
    }
}

new contract destroyable:

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

contract thisContractWillBeDestroyed is Destructible{
    // ...
}

[cincinnato]

import “./Ownable.sol”;

contract Destroyable is Ownable {
function close() public Ownable {
selfdestruct(msg.sender);
}
}

[FredrikLarsson]

pragma solidity 0.7.5;

contract Destroyable {
    
    address owner;
    
    constructor(address _owner) {
        owner = _owner;
    }
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    function destroy() public onlyOwner {
        selfdestruct(msg.sender);
    } 
}

[Haysoose]

pragma solidity 0.7.5;

contract Destructable{

address owner;

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

constructor(){
    owner = msg.sender;
}

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

}

[jon_m]

Hey @Jethrodog,

@Jethrodog

I noticed he had an extra line of code in his solution, where he has to create and set an address variable called receiver as payable and then initialize it as msg.sender, then use it in the selfdestruct call as the argument.
Is there any reason why he wouldn’t just put msg.sender in there and skip this step?

Good question! This should answer it …

@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

@Jethrodog

Here is the start of my Bank contract showing it inheriting the Destroyable:

pragma solidity 0.7.5;

import “./self_destruct.sol”;

contract Bank is Destroyable { …

That’s correct As you can see I’ve removed the interface from your post — I hope you don’t mind, but as that’s part of the next section I thought it might confuse other students who may be reading these posts for this assignment

[jon_m]

Hi @FredrikLarsson,

Can we also see how you’ve coded the start of your Bank contract to include the inheritance? The fact that you have a constructor in your parent contract that requires an address argument to be input on deployment means that you need to make an additional modification to the syntax of the Bank contract header which wasn’t covered in the video lectures. I’m interested to see if you worked out how to get your version to compile without any errors.

You can also make your code more modular by separating out the “destroy contract” functionality into a separate contract. You can then structure everything using multi-level inheritance:
Bank inherits Destroyable,
Destroyable inherits Ownable.
(Bank therefore inherits Ownable indirectly via Destroyable, because Destroyable already inherits Ownable).

At the moment, because you have combined both Ownable and Destroyable in one contract, if you don’t need to make owner available in the Bank contract, then you could also give owner private visibility. However, if you use Ownable and Destroyable as 2 separate contracts as I’ve suggested above, then owner will need internal visibility (either stated with the visibility keyword, or by default as you have it currently), because it will need to be inherited by Destroyable so it can be used in the selfdestruct function.

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

[jon_m]

Nice solution @Haysoose

You can also make your code more modular by separating out the “destroy contract” functionality into a separate contract. You can then structure everything using multi-level inheritance:
Bank inherits Destructable,
Destructable inherits Ownable.
(Bank therefore inherits Ownable indirectly via Destructable, because Destructable already inherits Ownable).

Can we also see how you’ve coded the start of your Bank contract to include the inheritance?

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

[Haysoose]

I’ve taken your suggestion and made the code more modular. The contracts below show a hierarchical inheritance structure wherein both Bank and Destructable inherit from Ownable.

Parent (Ownable)

pragma solidity 0.7.5;

contract Ownable{
    
    address internal owner;
    
    modifier onlyOwner {
        require(msg.sender == owner, "Must Be Owner");
        _;
    }
    
    constructor(){
        owner = msg.sender;
    }
    
}

Child 1 (Bank)

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Bank is Ownable {
    
    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, "Insufficient Balance");
        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, "Insufficient Balance");
        require(msg.sender != recipient, "Cannot Send To Self");
        
        uint prevBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == prevBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
}

Child 2 (Destructable)

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destructable is Ownable{
    
    function destroy() public onlyOwner {
        selfdestruct(msg.sender);
    }
    
}

[jon_m]

Hi @Haysoose,

Remember that we want to deploy the Bank contract and have it inherit from both Ownable and Destructable. This is because we want the owner of Bank to be able to destroy it if necessary. With your current hierarchical inheritance structure the owner can’t do this because the selfdestruct function is not inherited by Bank.
Your first version did provide a workable solution, but to make it more modular you need a multi-level inheritance structure instead.