Inheritance Assignment

created new contract Destroyable:

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

contract Destroyable is Ownable{
    
    function destroyContract() public onlyOwner {
        selfdestruct(msg.sender);
    }
}
```HelloWorld contract which inherits from Destructible contract.

pragma solidity 0.5.12;

import "./Destructible.sol";

contract HelloWorld is Destructible{...
}

You’re nearly there @Gene03, but your derived contract won’t compile because you seem to have got your contract and file names mixed up…

The name of your parent contract is Destroyable…

And it looks like the name of the file is Destructible.sol, because that’s what you’re importing …

But to successfully inherit the parent contract you need to include its contract name in the derived contract header, not its file name …

contract HelloWorld is Destroyable { ... }

and not

Are you not following the most recent Ethereum Smart Contract Programming 101 course, which uses Solidity v0.7.5? …

The old course used v0.5.12 and included a HelloWorld contract. Unless you are just finishing off the old course because you had already started it over a year ago, you should be following the course based on the more up-to-date syntax (Solidity v0.7), and which develops a Bank contract throughout the course (also used in the assignments) instead of HelloWorld.

Let me know if anything is unclear, or if you have any questions :slight_smile:

(By the way, to make sure we don’t get confused about which version we are discussing, I’ve deleted your first post containing just your Destroyable contract, because your second post contains what I assume is your updated version. I hope you don’t mind!)

Here is my solution, seem to work fine. Any mistakes? :slight_smile:

Ownable file:

contract Ownable {

address payable public owner;

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

constructor () {
    owner = msg.sender;
}

}

Destroyable file:

import “./Ownable.sol”;

contract Destroyable is Ownable {

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

}

Bank headers:
pragma solidity 0.7.5;

import “./Ownable.sol”;
import “./Destroyable.sol”;

contract Bank is Destroyable{

1 Like

Nice solution, which meets all of the assignment objects @antonfriedmann :muscle:

Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destroyable because it will implicitly inherit Ownable via Destroyable.

However, by coding your inheritance structure this way, Bank.sol only needs to import Destroyable.sol, so you can remove the line:  import "./Ownable.sol";

Let me know if you have any questions.

Inheritance Assignment

3 separate contracts

bank.sol

pragma solidity 0.7.5;

import “./Destroyable.sol”;

contract Bank is Destroyable{


ownerable

pragma solidity 0.7.5;

contract Ownerable{

//public will make owner to be seeable.
address payable public owner;

modifier onlyowner{
    require(msg.sender == owner);
    _; // run the functions.
}

 constructor(){
    owner = msg.sender;
}

}

destroyable.sol

pragma solidity 0.7.5;

import “./ownerable.sol”;

contract Destroyable is Ownerable {

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

}

1 Like

when i copy paste the code from the reading into remix, i get an error that “type address is not implicitly convertible to expected type address payable” at the line where “owner = msg.sender”
my initial solution was just to make the address not payable; does the address need to be payable?
then i read some feedback around here and found out that the “owner” variable isn’t even necessary, so I’d rather just use msg.sender directly unless there’s some need to abstract it into a variable…

back in a while with some attempted solutions…

edit to above: then i went back to the ownable contract itself (i.e. in a separate pane/tab); setting THAT “owner” to payable, resolved the issue in the destroyable contract! they’re looking at each other’s contracts within the same workspace! i had no idea…
THEN i reloaded the remix page and all errors went away… (i noticed the error log telling me i had something in there that was no longer there, so i reloaded the remix page itself and voila! no more errors!
then i changed the local variable anyway.

later edit:
my solution

“ownable_contract.sol”

pragma solidity 0.7.5 ;
contract ownable {
    address payable public owner;
    constructor() { 
        owner = msg.sender;
    }
    modifier onlyOwner() {
        require(msg.sender==owner, "only the owner can do that!");
        _;
    }
}

“make_destroyable.sol”

pragma solidity 0.7.5;
import "./ownable_contract.sol";
contract make_destroyable is ownable {
    function close() onlyOwner public { 
        selfdestruct(owner); 
    }
}

“bank” contract header:

pragma solidity 0.7.5 ;
import "./make_destroyable.sol"; 
contract testinglessons is make_destroyable {
...

in order to test it, i first compiled all three, deployed only the “bank” contract, and checked the following (in this order):
1 a non-permissioned (in this case, a ‘get’) function from the owner contract
2 a function permissioned as onlyOwner (in this case, a ‘‘set’ function)
3 switching accounts, and repeat the above two steps (expecting the latter to fail)
4 the ‘close’, i.e. ‘selfdestruct’, function from same account as step 3 (expecting it to fail, e.g. by checking that a non-permissioned function still works, still “getting” in this case)
5 switching back to first account (i.e. owner, i.e. msg.sender) and repeat first two steps to check that the contract wasn’t destroyed for only this address (is that even possible? …the way i code? maybe…ha!)
6 from same account trying the ‘close’ function again, this time expecting it to work (e.g. in this case, by checking ‘get’ then set’ then ‘get’ again expecting “0”, then no issue, then “0” again, as described in reading)

remaining questions:
in the solutions, address is not payable, and IS private; i think i understand privacy to be a safer default than ‘public’, but is there a meaningful effect here?
in the solutions, the order in which “pragma…” is listed with respect to “import…” seems not to matter; do i understand correctly? (similarly there is no “pragma…” in the “ownable.sol” solution on github; is it unnecessary because the compiler will only be used on the most derived contract?)

1 Like

Destroyable file

pragma solidity 0.7.5;

import "./Ownable.sol";

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

Ownable file

pragma solidity 0.7.5;

contract Ownable {
    
    address payable public owner;

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

and partially the bank file, modified only by inheriting from Destroyable and adding import to Destroyable

pragma solidity 0.7.5;

import "./Destroyable.sol";

contract Bank is Destroyable {
1 Like

Excellent @Gry :muscle:

Excellent solution @michael356 :raised_hands:

Make sure you format ALL your code before posting it; not just bits of it. :wink: When it’s not all formatted it’s harder to review.

Excellent solution @B_S :muscle:

I’ve followed through your testing, and apart from the first “getter”, I assume you are then calling the deposit() and getBalance() functions — and perhaps performing a transfer as well — in Bank. Is that correct? You need to ensure you deposit some ether in the contract in order to test that, when selfdestruct() is called, the ether remaining in the contract is transferred to the contract owner’s external address.

I also find it helpful to add an additional getter to the Bank contract, to check the total contract balance at various stages (especially just before selfdestruct is called) e.g.

function getContractBalance() public view returns(uint) {
    return address(this).balance;
}

If selfdestruct() is executed successfully, it removes the contract from the blockchain, and so it can no longer be interacted with by any address.

When you checked these functions after calling close() with the owner address, you should have noticed that, despite the contract having been destroyed, the function calls were still successfull and didn’t throw errors! We can be sure that the contract has been destroyed, because the getters now return zero values. However, a clear disadvantage with using selfdestruct() is that, after the contract has been destroyed, a payable function can still be called, by mistake, with an ether value and, instead of reverting, the ether will be lost. Therefore, if a smart contract deployed on the mainnet is ever destroyed, all users need to be notified and informed that they must not send any more funds to that contract address.

After destroying the contract, if you call the deposit() function with an ether value, you will see that the ether is deducted from the caller’s external address. If you then call getBalance() or getContractBalance(), either of these will return zero.

The potential repercussions of selfdestruct need to be fully understood before choosing to implement it; but for the purposes of this introductory course, it provides a simple and straightforward way to practise implementing inheritance, which is our main objective in this assignment. More effective alternatives exist, such as pausable contracts. These involve more advanced techniques and are therefore out of the scope of this introductory course, but you can learn about pausable contracts and other smart-contract emergency and security features, such as proxy contracts, in the Smart Contract Security course.


Remaining Questions

I assume you’re referring to the owner state variable in Ownable…

  • The solution provided is just one of several alternatives. It marks the owner variable with internal (not private) visibility. In fact, state variables are internal by default, so owner will still have internal visibility if the keyword is omitted. State variables and functions with internal visibility are inherited, but are only available within the contract and its derived contract(s); they cannot be accessed externally. The only effect you would see by changing the visibility of owner from public to internal is that the getter (to retrieve the owner’s address) would no longer be automatically generated.
    If a state variable or function is marked with private visibility, then it can only be accessed from within the same contract (excluding any derived contracts). We should always aim to restrict access to only what is necessary; this increases security and, as you say, is safer.

  • In the solution, the owner state variable isn’t declared as a payable address type because, in Destroyable, selfdestruct() is effectively called with msg.sender (via an intermediary local variable receiver), instead of owner. Prior to Solidity v0.8 msg.sender is payable by default. We can use msg.sender to reference the contract owner’s address, because this is the only address that it can ever represent in the close() function, as a result of the onlyOwner modifier restricting who can access it.
    If we only need the contract owner’s address to be payable for the purposes of executing selfdestruct(), another alternative solution is to leave the owner state variable as non-payable, and explicitly convert the address to payable locally, within the function where we actually need to use it as payable. We can even do this directly within the selfdestruct() function call itself, as follows:

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

Yes … the contract will still compile and deploy successfully using either order. However, personally, I prefer to place any import statements directly beneath the pragma statement, as you have done.

This is a good question…
It is not good practice to omit the pragma statement from any Solidity file, because we should always define the Solidity compiler version (or range of versions) which our code is written to be compatible with.
Because the Bank contract inherits functionality from the Destroyable and Ownable contracts, even if the pragma statements are omitted from either or even both of these inherited contracts, Remix will still attempt to compile Bank as a single contract together with the code it inherits. Bank will compile and deploy successfully as long as the code in both of the inherited contracts is compatible with the Solidity version declared in the pragma statement at the top of the file containing Bank. However, the compiler still issues orange/yellow warnings, highlighting that it’s best practice to include pragma statements in each file.
Having experimented a bit, it seems that, in the absense of a pragma statement, Remix will always try to find a suitable compiler version, although this doesn’t seem very reliable.
In conslusion: always include a pragma statement at the top of every .sol file.

//Destroyable contract:

import “./Ownable.sol”;

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

//In Bank contract
pragma solidity 0.7.5;

import “./Ownable.sol”;
import “./Destroyable”;

contract HelloWorld is Destroyable {

1 Like

Below is my solution for the inheritance assignment.

One Question:
In my solution the Bank contract inherits from the other two contracts. However, the Ownable contract is parent to the Destroyable contract, and the Destroyable contract is parent to the Bank contract. Thus the Bank contract inherits from the Ownable contract through the Destroyable contract. Is this statement correct?
If yes, should I eliminate the “is Ownable” in the Bank contract because it is redundant or should I keep the “is Ownable” in the code because it has no negative effect (e.g. performance) and it actually makes it much easier to detect the inheritance relationships for anyone who reads the code?

File bankOwnable.sol

pragma solidity 0.5.7;

import "./ownable.sol";
import "./bankOwnableDestruction.sol";

contract Bank is Ownable, Destroyable{
...
}

File Ownable.sol

pragma solidity 0.5.7;

contract Ownable{
    
    address payable public owner; // "paybale" makes address eligible to receive funds; "public" makes state variable eligible for inheritance
    
    modifier onlyOwner{
        require(msg.sender == owner);
        _;
    }
 
    constructor() public {
        owner = msg.sender;
    }
}

File bankOwnableDestruction.sol

pragma solidity 0.5.7;

import "./ownable.sol";

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

i think that’s correct; after the first getter function, i just used a quick and dirty ‘set’ function to “transfer” funds. good point about checking the remaining ether; i hadn’t done so.

yes! saw that

payable takes arguments?! … so many things i have yet to learn…good thing i don’t intend to actually use this for my bread and butter yet ha!

1 Like
payable(owner)

This isn’t a function call, it’s just convenient syntax to convert a non-payable address, which is placed within the parentheses, to a payable one. In Solidity, parentheses can be used in this way to perform certain explicit type conversions e.g.

uint256 x = 50;
uint8 y = uint8(x);

string memory message = "Hello World!";
bytes memory z = bytes(message);

uint160 a = uint160(msg.sender);
address b = address(a);

bytes20  c = bytes20(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4);
address d = address(c);

Relevent references in the Solidity Documentation
https://docs.soliditylang.org/en/latest/types.html#address
https://docs.soliditylang.org/en/latest/types.html#explicit-conversions

1 Like

Hi @B_S,

I’ve just written this contract, which you can play around with. It uses the same explicit type conversions as my example in my last post. But this time you can output the converted values.

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.10;

contract TypeConversion {
    
    uint8 y;
    bytes z;
    uint160 a;
    address b;
    bytes20 c;
    address d;

    function convertType() external {
        uint256 x = 50;
        y = uint8(x);

        string memory message = "Hello World!";
        z = bytes(message);

        a = uint160(msg.sender);
        b = address(a);

        c = bytes20(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4);
        d = address(c);
    }

    function getResults() external view returns(uint8, bytes memory, uint160, address, bytes20, address) {
        return (y, z, a, b, c, d);
    }
}
1 Like

Hi @Matias_Haller,

Good to see you back here in the forum :smiley:

Your solution is almost there; but it won’t compile because you’re not importing the Destroyable.sol file…

…you’re missing the .sol file extension.

Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. HelloWorld only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. However, this also means that you don’t need to import Ownable.sol into HelloWorld.sol (just Destroyable.sol)…

What happened to Bank? :wink:

Let me know if you have any questions.

1 Like

Hi again @Matias_Haller,

I’ve also noticed that you don’t seem to have posted any of the other assignments in this course. Even if you’ve watched and coded along with all of the videos, I would still encourage you to complete and submit the assignments, because these provide important practice and as such are an important part of the course. If you don’t get enough practice, you may struggle with the 201 course which follows this one.

Also, don’t forget to format your code before posting it. This makes it clearer, easier to read, and easier to spot any syntax errors. It also makes it easier to copy and paste if we need to deploy and test it.
Follow the instructions in this FAQ: How to post code in the forum

Just let me know if you have any questions :slight_smile:

1 Like

Hey Alex,

Your solution is almost there… Everything compiles, your inheritance structure works, the contract can be destroyed by calling the close() function in Destroyable, and on destruction any ether still remaining in the contract will be transferred to the contract owner’s external address :ok_hand: You just have one problem to resolve…

At the moment anyone can call close() and destroy the contract. The funds are safe, because only the contract owner can receive them. But the fact that any address can actually trigger selfdestruct is obviously not ideal :sweat_smile:

What functionality, inherited from Ownable, can you implement in Destroyable, to ensure that only the contract owner can call close() and trigger selfdestruct ?


Yes … this is correct…
Your implementation works, but there is no need to include the code for Bank to explicitly inherit Ownable. A multi-level inheritance structure is the best approach, and this is what you’ve described in your statement. Bank only needs to explicitly inherit Destroyable because it will implicitly inherit Ownable via Destroyable.

Yes… that’s correct — it’s redundant.

Your solution still works perfectly well, and you are right that it has no negative effect on performance.

I would actually say that using multi-level inheritance produces cleaner and clearer code, because it is more streamlined. By removing the redundant parent-child relationship, the true three-level, linear relationship is better reflected in the code.

As well as removing this, you should also remove the associated import statement…


A couple of other observations …

What’s happened to v0.7.5 ? Are you deliberately experimenting with the older v0.5, or is this just a slip? :wink: You probably noticed that the v0.5 syntax for the constructor was slightly different: it required visibility to be defined as either public or internal. From v0.7 visibility no longer needs to be defined for constructors.

You could also define this state variable with internal visibility and it would still be inherited. It would then still be available within the derived contract(s), but not externally, and the automatic getter would no longer be created. With state variables, internal visibility is the default, so the keyword doesn’t need to be included…

// even though not stated, this state variable has internal visibility
address payable owner;

With functions, however, visibility must always be explicitly defined.

Let me know if you have any further questions :slight_smile:

1 Like

As always, thanks a lot for your input Jon :slight_smile:

First of all, to answer some of your remarks:

  • I mixed up the numbers of the version => of course I am using v0.7.5 :smiley:
  • Your comments concerning the multi-level inheritance are well understood and I adjusted my code accordingly
  • I also understood your comments regarding the variable declaration (for the variable owner)
  • I realized what my mistake was with the modification of my close() function and I adjusted it accordingly by inserting the onlyOwner modifier.

My adjusted solution:

File bankOwnable.sol

pragma solidity 0.7.5;

import "./bankOwnableDestruction.sol";

contract Bank is Destroyable{
...
}

File ownable.sol

pragma solidity 0.7.5;

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



File bankOwnableDestruction.sol

pragma solidity 0.7.5;

import "./ownable.sol";

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

I have another question concerning the meaning of msg.sender:

  • The mistake with the onlyOwner modifier in my Destroyable contract is actually the result of a misconception about the meaning of msg.sender
  • From the tutorial of this academy and also from a written documentation (See “Block and Transaction Properties”), I assumed that msg.sender is equal to the current address that interacts with the contract, or in other words the address of the entity that calls the contract and its functions etc. I think this statement is correct, isn’t it?
  • My misunderstanding was that I was mislead by the functionality of the Ownable conctract in the way that I assumed each time another address interacts with the contract, the constructor changes the value of the variable owner to the address of the party currently interacting with the contract. In the moment of writing the Ownable contract, I just missed the important fact that the constructor is executed only once when the contract is deployed. Since I do not change the variable owner somewhere else, it remains equal to the address of the party that deploys the contract.

To sum it up, are the following statements correct?
1.) msg.sender equals the address of the party/entity that currently interacts with the conctract (applies to both, the party that initially deployed the contract and all other parties that interact with the contract after its deployment)
2.) Using the Ownable contract, as shown above, is a best practice to store the address of the party that deployed the contract and can then be used for different purposes (in this case to selfdestruct the contract).

A final point:
I totally agree with the safety aspect related to selfdestruct - in case a smart contract has vulnerable code that is being exploited by an attacker, the designated contract owner can withdraw all remaining funds from the contract and mitigate the financial damage.
However, having one address with the power to withdraw all funds from a contract represents a strong concentration of power. I assume it is common practice that initiating the function to trigger selfdestruct requires the signature of multiple parties, but still the power remains in the hand of e.g. a few members of the developer team. My question actually is, is this a big/hot topic in the crypto space (the good old story about security vs. decentralization) and what is the general perception about this issue (feel free to post your own opinion if you want :slight_smile: )

1 Like

Hey, it’s my turn :stuck_out_tongue:

First, a “mistake” in the assignment I think : “Then, you should be able to deploy the Bank contract, perform some transactions and, finally, destroy the contract, transferring any remaining ether held in the contract to the contract owner.”

In fact, in our example, there is “owner = msg.sender” in Ownable class. That means, any remaining ether in balance[msg.sender] will be always equals to balance of the owner cause this is the same address.

So, my code below don’t work, because the transfer method try to transfer remaining ETH from and to the same balance. I think we have to rewrite lots of logic in order to perform a scenario where the msg.sender and owner will be different in order to check if the transfer of remainin ETH will work. I will see in the assignement solution what it will.

The good side of this “mistake”, if it is, I know use the debugger with Remix.

DESTROYABLE.SOL

pragma solidity 0.7.5;

import "./Ownable.sol";


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

OWNABLE.SOL

pragma solidity 0.7.5;

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

BANK.SOL

pragma solidity 0.7.5;

import "./Destroyable.sol";


contract Bank is Destroyable
{
    mapping(address => uint) balance;

    
    //EVENTS
    event balanceAdded(uint amount, address indexed depositedTo);
    event depositDone(uint value, address indexed sender);
    event closureDone(uint value, address indexed sender);
    
    //CLOSURE
    function closeWalletBasic() public
    {
        uint senderBalance = getSenderBalance();
        
        if(senderBalance > 0)
        {
            transfer(owner, senderBalance);   
            
            assert(balance[msg.sender] == 0);
        }
        
        close();
        emit closureDone(balance[msg.sender], msg.sender);
    }