Inheritance Assignment

Mark_Dalton · January 17, 2021, 9:51am

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{

    // delete the deployed contract
    function close() public onlyOwner {
        selfdestruct(owner);
    }
}

nick_bell2017 · January 17, 2021, 2:43pm

selfDestrucr.sol

pragma solidity 0.7.5;

import "./ownable.sol";

contract Destroyable is Ownable {
    
    function close() public onlyOwner { //onlyOwner is custom modifier
        selfdestruct(msg.sender);  // `owner` is the owners address
    }
}

and in Helloworld.sol

import "./selfDestruct.sol";

contract BankOfNico is Ownable, Destroyable

Mirran · January 17, 2021, 6:02pm

pragma solidity 0.7.5;
import “Ownable.sol”;

contract Destroyable is Ownable {

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

}

/* to use
import “Destroyable.sol”;
contract HelloWorld is Destroyable{
}
*/

codinginlondon · January 17, 2021, 6:42pm

Here is an example of Destroyable base contract that allows the Bank contract to be destroyed, provided the caller is the owner.

It seems to work but I am a bit confused because Remix allows me to call “destroy” more than once. The second call does not fail but has a transaction cost of zero.

pragma solidity 0.7.5;



contract Destroyable{
    
    address payable owner;
    
    constructor(){
        owner = msg.sender;
    }
    
     function destroy() public {
        require( msg.sender == owner, "Only the owner can destroy this contract");
        selfdestruct(owner);
    }
   
}

contract Bank is Destroyable{
    
    
    mapping(address => uint) balance;
    
    
    event depositDone(uint amount, address indexed depositedTo);
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }

    
    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 returns (uint){
        require(balance[msg.sender] >= amount, "balance not sufficient");
        msg.sender.transfer(amount);
    
        balance[msg.sender] -= amount;
        
        return balance[msg.sender];
    }
    
    function getBalance() public view returns (uint){
        return balance[msg.sender];
    }
}

ZigaP · January 17, 2021, 11:29pm

This is my solution to the problem. I changed the main smart contract to “is Destroyable”. I noticed that I got an error at the input of selfdestruct(owner)… I thought the variable owner is accessible, since it is public, but I noticed from the way others solved this, that I have to put msg.sender. Could you explain why is this? Since I’ve made the Destroyable contract “is Ownable”, shouldn’t it by rule of inheritance know what the variable “owner” is? Thank you very much, for the answer.

contract Ownable {
    
    address public owner;

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

    constructor(){
        owner = msg.sender;
    }
}

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

jon_m · January 17, 2021, 11:50pm

Hi @codinginlondon,

Good question! Your contract does work, but you are correct that even after the contract has been destroyed, the destroy function can still be called. In fact any of the other functions can also still be called, and gas will be consumed. However, the smart contract is inoperable and even though the functions can still be invoked they won’t have any effect (other than consuming gas). But the fact that users are not made aware that the smart contract has been destroyed, and can still make function calls that consume gas, is obviously a disadvantage with using selfdestruct. This is why selfdestruct is only meant to be invoked in extreme circumstances e.g. to retrieve funds held in the contract in the event of an attack, as a form of damage limitation. One way in which this disadvantage with selfdestruct() can be avoided is by adding code that will divert control of the contract in the event of selfdestruct being invoked.

One way in which your contract can be improved is to move the onlyOwner modifier from the derived contract to the parent contract. That way it can be used to restrict access to selfdestruct() in the parent contract, while still being available via inheritance in the derived contract, if needed. This avoids code duplication and the need to repeat the same require statement in both the modifier and the destroy function.

Also, have a look at this post which explains an important security modification that should be made to the order of the statements within your withdraw function body.

jon_m · January 18, 2021, 12:13am

Hi @ZigaP,

Good question! You are correct that as the state variable owner has public visibility it is inherited and accessible in the derived contract Destroyable. However the reason you are getting the error is not because of this.

The address argument passed to selfdestruct() is where the contract’s ETH balance is transferred to on destruction (so these funds aren’t lost). Because this address is used to receive a payment, it needs to be a payable address.

Addresses can be stored in variables as either payable, or non-payable. Unless we specifically mark them as payable, then they are non-payable by default. However, when msg.sender is used directly as a value (and not first assigned to a variable), it is treated as a payable address by default in Solidity, so that’s why selfdestruct(msg.sender) works without having to add anything further or make any additional modifications. If you do the same with owner the compiler will give you an error, because you currently have it defined as a non-payable address in contract Ownable:

address public owner;

You can still use owner but you will need to also define it as a payable address by explicity adding the payable keyword:

address payable public owner;

If you make this modification to the owner state variable definition in Ownable, then selfdestruct(owner); will work.

Another alternative is to leave the owner state variable definition as non-payable, and convert it to a payable address directly within the selfdestruct function call in contract Destroyable:

selfdestruct(payable(owner));

I hope that makes sense now, but do let us know if you have any further questions

Nelson1 · January 18, 2021, 5:55am

I got it to work!..
3 files, 1 The contract to be destroy, 2 The Ownable, 3 The Destroyable

Code for the Contract to be Destroyed…
pragma solidity 0.7.5;
import “./Ownable.sol”;
import “./Destroyable.sol”;

contract Bank is Ownable, Destroyable {
mapping(address => uint) Balance;

function addBalance(uint _ToAdd) public onlyowner returns(uint){…}

}

Ownable file…
pragma solidity 0.7.5;
contract Ownable {
address owner;
modifier onlyowner {
require(msg.sender == owner, “only owner can run this function…”);
_;
}
//We use constructor to initialize variables…
constructor(){
owner = msg.sender;
}
}
The Destroyable contract…

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

contract Destroyable is Ownable {

function close() public onlyowner{
    
    selfdestruct(msg.sender) 
}

}

ZigaP · January 18, 2021, 8:56am

Cleared up thanks. Below should just be the correct answer, should anyone find it useful.

In order to solve the problem we have to somehow let the contract be accessed by onlyOwner and we have to make the address where funds are being withdrawn to, somehow payable. In the solution below, the first characteristic is derived from inheritance, while for making variable payable there are multiple options:
Option #1 - we make the address payable by using msg.sender

contract Ownable {
    
    address public owner;

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

    constructor(){
        owner = msg.sender;
    }
}

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

Option #2.1 - we make the owner variable payable by hand

contract Ownable {
    
    address payable public owner;

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

    constructor(){
        owner = msg.sender;
    }
}

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

Option #2.2 - we can also leave the state variable owner as non-payable and instead making it payable only locally within the Destroyable function, like below.

address public owner;
...
function destroyContract() public onlyOwner {
    selfdestruct(payable(owner));
}

jiadong_han · January 18, 2021, 3:46pm

Ownable file:

pragma solidity 0.7.5;
contract Ownable{

 address owner;

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

}

constructor(){

  owner = msg.sender;

}
}

Destroyable file:
import “./Ownable.sol”;
pragma solidity 0.7.5;
contract Destroyable is Ownable{

  function close() public OnlyOwner { //onlyOwner is custom modifier
     selfdestruct(msg.sender);  // msg.sender is the owners address by the father contract Ownable

}
}

Bank file:
import “./Ownable.sol”;
import “./Destroyable.sol”;
pragma solidity 0.7.5;
contract bank is Ownable, Destroyable{
}

codinginlondon · January 18, 2021, 7:00pm

Thanks Jon, this is very useful info!
So selfdestruct() is more of a last-resort kill switch. And we should design contracts keeping in mind that they will exist forever on the chain.

Cheers,
Matt

Conor_Neary · January 18, 2021, 9:57pm

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

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

MindFrac · January 19, 2021, 2:59am

Cool it works!

**Ownable.sol**

pragma solidity 0.7.5;

contract Ownable {
    address payable public owner;

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

**Destroyable.sol**

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {

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

}

herbz · January 19, 2021, 11:11am

Ownable.sol

contract Ownable {
    address payable owner;

    constructor(){
        owner = msg.sender;
    }
    
    modifier onlyOwner{
        require(msg.sender == owner);  
        _;
    }
    
    function getOwner() public view returns (address){
        return owner;
    }
}

Destroyable.sol

import "./Ownable.sol";

contract Destroyable is Ownable{
    
    function close() public onlyOwner { //onlyOwner is custom modifier
        selfdestruct(owner);  // `owner` is the owners address
    }
}

jon_m · January 19, 2021, 11:21pm

Nice solution @Nelson1

We can also streamline the inheritance by having Bank just inherit Destroyable, because it will then indirectly inherit Ownable via Destroyable. This will give you a multi-level inheritance structure. There would also no longer be any need to import Ownable.sol into Bank.sol (just Destroyable.sol).

jon_m · January 19, 2021, 11:31pm

Nice solution @jiadong_han

We can also streamline the inheritance by having Bank just inherit Destroyable, because it will then indirectly inherit Ownable via Destroyable. This will give you a multi-level inheritance structure. There would also no longer be any need to import Ownable.sol into Bank.sol (just Destroyable.sol).

jon_m · January 20, 2021, 12:12am

Exactly right.

The data relating to transactions which have already been executed and validated using the smart contract will remain immutable and stored permanently on the blockchain. We should design our smart contracts based on the premise that once they are deployed they can’t be modified. This means building them with a rigorous attention to security and only deploying them after thorough testing. The smart contracts themselves run on the EVM, which isn’t actually the same as the Ethereum blockchain. This is an important distiction to understand. The blockchain itself is where all the data is stored.

Believing that a smart contract will exist and run forever on the EVM is almost certainly unrealistic, as future developments in technology will eventually render a smart contract, which by it’s very nature can’t be modified or updated, either inefficient or obsolete, or increasingly at risk from ever more sophisticated attacks.

One way to manage the risk of a smart contract eventually being compromised or becoming obsolete, and to limit the potential financial loss that this could lead to, is to include the selfdestruct functionality.

jon_m · January 20, 2021, 12:27am

Hi @Conor_Neary,

You’ve nearly got it, but because you’ve called selfdestruct with owner as the argument, you need to make it a payable address. You should have got a compiler error as result of this.

Have a read of this post which explains the issue and the various ways to resolve it.

Also, what modifications did you make to the Bank contract header for the inheritance?

jon_m · January 20, 2021, 12:32am

Excellent coding @MindFrac

What modifications did you also make to the Bank contract header for the inheritance?

jon_m · January 20, 2021, 12:41am

Nice solution @herbz

Don’t forget to add pragma solidity 0.7.5; at the top of each file. Your contract won’t deploy without it.

Also, what modifications did you make to the Bank contract header for the inheritance?