Inheritance Assignment

[jon_m]

An excellent solution, Dustin!

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.

Your progress is becoming exponential!

And thanks for letting me know that the feedback has been helpful… that’s always good to hear

[jon_m]

Hi Denis,

Overall, this is a very good solution which achieves everything that it should

Your well-documented thought process is also very logical, shows that you have a good understanding of what your code is achieving, and that you’ve approached the task in a suitable way.

Your initial diagram is spot on in terms of the inheritance structure. More specifically, what you have illustrated and described is in fact a multi-level inheritance structure. Your coding of the inheritance is correct and works perfectly well, but we can also “streamline” the code by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable.

Your close() function does everything that it needs to, but the onlyOwner modifier is all that is needed to ensure that only the contract owner can call this function and trigger selfdestruct. This modifer contains a require statement with exactly the same condition as the one you’ve included with the assert() statement, which makes the assert statement redundant and best removed. In any case, require statements should be used to restrict access to functions (in addition to validating inputs) and not assert statements. Assert statements should be used to check invariances in the computations performed by the code within the function body.

Well done for realising that the address passed to selfdestruct() needs to be a payable address, in order for the remaining contract balance to be transferred to that address when the contract is destroyed. You are also right that we can use msg.sender to reference the contract owner’s address, because this is the only address that it can ever represent (as a result of the onlyOwner modifier restricting who can call the close function in the first place).

As always, just let me know if you have any questions

[crypto-djent76]

Hey @jon_m as always, thanks again for your insightful and helpful feedback. It makes sense once I read over it a couple of times.
I have adjusted the code as requested, and after reviewing, I agree that the assert() statement wasn’t required as onlyOwner modifier already has the require statement for allowing only the owner to modify the self destruct function.

Also, I see that the Bank explicitly inherits Ownable via Destroyable, so the inheriting of Ownable via Bank is redundant.
A question I had was, do I need the import statement “Ownable.sol” in Bank if indeed Bank will inherit Ownable via Destroyable, or is that made redundant as well?

Thanks again Jon. I appreciate your assistance as always

[jon_m]

Hey Denis,

I’m glad my comments make sense. That’s really good that you are carefully thinking through their implications, and working out for yourself why they improve your code — that’s the best approach to take

@crypto-djent76

do I need the import statement “Ownable.sol” in Bank if indeed Bank will inherit Ownable via Destroyable, or is that made redundant as well?

The import statement isn’t needed either. Both the importing and the inheriting of Ownable happen implicitly via Destroyable.
When you’re not sure about these sorts of things (which happens all the time) see if the compiler allows the modification you’re not sure about, or whether it generates any errors or warnings. This is normally a good first step to take in working out for yourself whether something works or not.

[crypto-djent76]

Thank you again @jon_m and as always, great advice.

I will indeed check the compiler to see if any errors are thrown that I can adjust myself before querying it.

Still a long way to go for me, but for now, I am slowly learning better how to understand the code.

[Joey]

Hi All,
I struggled abit to get it right with the owner variable.
here is my solution, please someone inform if I got it wrong:

pragma solidity 0.7.5;

import "./ownable.sol";

contract destroyable is ownable {
    
    function close() public onlyOwner {
        selfdestruct(owner);
    }
}
    

I couldn’t add the “owner” variable to work in the selfdestruct() function. the error kept reffering to a payable function.
so I added payable to the owner variable inside the ownable contract. See owner contract below:

pragma solidity 0.7.5;


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

it took me a moment to find it but i still dont understand why i had to make the variable payable.

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

contract bank is ownable, destroyable {
...

[szaslavsky]

pragma solidity 0.7.5;

contract Ownable {

address owner;


constructor() {
   owner = msg.sender;
}

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

}

---

pragma solidity 0.7.5;

import “./Ownable.sol”;

contract Destroyable is Ownable{

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

}

---

pragma solidity 0.7.5;

import “./Destroyable.sol”;

contract Bank is Destroyable {

mapping(address => uint) balance;

function deposit() public payable returns (uint) {
    balance[msg.sender] += msg.value;
    return balance[msg.sender];
}

function withdraw(uint amount) public returns(uint) {
    require(balance[msg.sender] >= amount);
    msg.sender.transfer(amount);
    balance[msg.sender] -= amount;
    return balance[msg.sender];
}

function getBalance() public view returns(uint) {
    return balance[msg.sender];
}

function transfer(address recipient, uint amount) public {
    require (balance[msg.sender] >= amount);
    require (msg.sender != recipient);
    balance[msg.sender] -= amount;
    balance[recipient] += amount;
}

}

[jon_m]

Hi @Joey,

First of all, apologies for the delay in giving you some feedback on your solution to this assignment, and in answering your specific question.

@Joey

here is my solution, please someone inform if I got it wrong:

No… your solution is correct

@Joey

I couldn’t add the “owner” variable to work in the selfdestruct() function. the error kept reffering to a payable function.
so I added payable to the owner variable inside the ownable contract.

Well done for using the error message generated by the complier to resolve the issue with the owner address not being payable. But note that it would have referred to the address argument passed to selfdestruct() needing to be payable, and not the function. The address argument passed to selfdestruct() needs to be a payable address, in order for the contract’s remaining ether balance to be transferred to that address when the contract is destroyed. To receive ether an address must be payable. There are several ways we can solve this:

(1) By doing what you have done, and declaring the owner state variable in Ownable with a payable address type, instead of with the default, non-payable address type.

(2) If you only need this address to be payable for the purposes of executing selfdestruct(), you can leave the owner state variable as non-payable, and explicitly convert the address to payable locally, within the function where you actually need to use it as payable. You can even do this directly within the selfdestruct() function call itself, as follows:

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

(3) We can also use msg.sender to reference the contract owner’s address, because this is the only address that it can ever represent (as a result of the onlyOwner modifier restricting who can call the close function in the first place). Prior to Solidity v.0.8, msg.sender is payable by default, and so you wouldn’t need to explicitly convert it…

selfdestruct(msg.sender);

From v.0.8, msg.sender is non-payable by default, and so whenever you are using v.0.8 and need to use msg.sender as a payable address, this requires an explicit conversion e.g.

selfdestruct(payable(msg.sender));

---

We can also streamline the inheritance structure by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

And just one other observation…

@Joey

contract bank is ownable, destroyable {

Normally we always start the names we give to contracts, structs, events and interfaces with a capital letter. Our code will still compile if we don’t do this, but it is easier to read and manage code when this and other conventions are applied consistently.

Just let me know if anything is still unclear, or if you have any further questions

[jon_m]

An excellent assignment solution @szaslavsky

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.

Just one thing that I’ve noticed… your withdraw() function now has the following two statements in a different order to what you originally had in your Transfer Assignment solution.
.

@szaslavsky

msg.sender.transfer(amount);
balance[msg.sender] -= amount;

It is more secure to have these in the opposite order, for the reasons I’ve explained in my feedback for your Transfer Assignment here.

You’re making great progress!

[mvrpraveen]

Following is my solution to the inheritance assignment:

contract Ownable:

pragma solidity ^0.7.6;

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

contract destroyable:

pragma solidity ^0.7.6;

import "./ownable.sol";

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

contract Bank:

pragma solidity ^0.7.6;

import "./destroyable.sol";

contract Bank is Destroyable {

[Joey]

Thanks for the feedback and help Jon. I’m on the multysig wallet assignment now.

[Random]

Thank you very much @jon_m appreciate your replies to my questions

[KennethJP]

Ownable.sol

pragma solidity 0.7.5;

contract ownable {
    address 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 {
    
    function close() public onlyOwner { 
  selfdestruct(msg.sender); 
 }
    
    
}

Bank contract beginning
pragma solidity 0.7.5;

import “./Destroyable.sol”;
contract bank is Destroyable {

[AngSin17]

HelloWorld.sol:

pragma solidity 0.7.5;

import './Destroyable.sol';

contract Bank is Destroyable {

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;
    }
    
    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }
}

[HemalT]

pragma solidity 0.5.12;

contract Destroyable{

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

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

[2tijan7]

Here is my Bank contract:

pragma solidity 0.7.5;

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

contract EtherBank is Ownable, Destroyable {

Ownable contract:

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

Destroyable contract:

import "./Ownable.sol";

contract Destroyable is Ownable{

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

[DavidV]

Contract 1
pragma solidity 0.7.5;

contract Ownable {

address public owner;
constructor(){
   owner = msg.sender;

}
modifier onlyOwner {
require(msg.sender == owner,“NOT THE OWNER!”);
_; //run the function;
}
}

Contract 2

pragma solidity 0.7.5;

import “./Ownable.sol”;

contract Destroyable is Ownable {

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

}

// BTW why could I not use “owner” in the Selfdestruct function?

Contract 3 :

pragma solidity 0.7.5;

import “./Destroyable.sol”;

contract CryptoBank is Destroyable{

[DavidV]

why does the Bank code need to inherited both Ownable and Destroyable? Ownable is already a child of Ownable? Or am I missing how this works. My toe worked fine without Ownable…

[Gomez1333]

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

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

Destryoable contract
pragma solidity 0.7.5;

import “./Ownable.sol”;

contract Destroyable is Ownable {

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

Bank contract - beginning
pragma solidity 0.7.5;

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

contract Bank is Ownable,Destroyable {
…(rest is unchanged)

[jtb]

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

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