Inheritance Assignment

[Austin1]

import “Ownable.sol”;
pragma solidity 0.5.12;

contract Destoryable is Ownable{

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

}

[jon_m]

Hi @hannezzon,

Nice solution, and accompanying comments and analysis
Don’t forget you also need to import the inherited contract’s file into the derived contract with:

// in HelloWorld.sol
import "./Destroyable.sol";

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

… although I’m sure you have this in your full code in Remix, and it’s just been missed from the summary of your additions and changes

@hannezzon

Question on your solution: The msg.sender is the owner, but it is only implied by onlyOwner doing its job flawlessly. It should work and I do not see how to “hack” to steal funds, but just in case would it not be a notch safer to use owner in Ownable as receiver instead? Or does it open up other weakness?

@hannezzon

would it not be a notch safer to use owner in Ownable as receiver instead?

I can see your point and understand your reasoning, and I certainly think you could make a case for using owner instead of msg.sender for added security. But I think we can view the following as equally valid and adequate solutions:

function destroyContract() public onlyOwner {
    // selfdestruct(address(uint160(owner)));
    // selfdestruct(msg.sender);
}

Note that with the following solution, you would also need to make address public owner; in contract Ownable a payable address: address payable public owner;

@hannezzon

function destroy() public onlyOwner {
    address payable receiver = owner;
    selfdestruct(receiver);
}

You may also be interested to read this post about how it isn’t actually necessary to include the additional step of saving owner or msg.sender to a local variable (here, receiver), but where Filip also explains why this additional step is included in the solution given in the video.

[jon_m]

@MarcisB

After posting that I realized that this is enough:

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

contract HelloWorld is Destroyable{

as Destroyable already inherits Ownable.

Absolutely, right! Well done, for realising that

@MarcisB

It is anyway good to keep code short, but would it also save gas?

Excellent question! I would expect there to be a small saving. Why don’t you try testing that, and then post your findings. It would be interesting to see how significant (or insignificant) any saving actually is

[jon_m]

Hi @Marcus_Tang,

Nice solution

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.

[jon_m]

Hi @Austin1

Nice solution

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

[Proniss]

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

contract Destructible is Ownable{

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

}

[jon_m]

Hi @Proniss,

Your code for contract Destructible provides a correct solution, but it involves:

1. making an additional change to contract Ownable;
2. amending contract HelloWorld in terms of inheritance.

Can you post these additional parts to your solution, so that we can see the complete picture?

(You don’t need to post the complete code for the other two contracts — just the lines with modifications)

[Proniss]

Hello @jon_m

looks like I couldn’t actually get this working, I couldn’t get it to work. It set all of the values to 0 but the contract still appeared to work. after messing around with a few other methods I was able to get it working the same way he did in the video. Also it seems Remix doesn’t save my files after I close browser, so unfortunately that code is lost forever. I could try to recreate if you want

[jon_m]

Hi @Proniss

@Proniss

Also it seems Remix doesn’t save my files after I close browser, so unfortunately that code is lost forever.

Oh, that’s annoying for you It must be something to do with your browser settings. My browser saves my Remix files, but I’m not sure how to fix it if it doesn’t. I’m tagging a few colleagues, as one of them may be able to offer some advice @Taha, @thecil, @gabba.

@Proniss

I could try to recreate if you want

Don’t worry, if it’s a hassle. It was a suggestion for your benefit, but if you’ve understood the solution that Filip explains in the video, and then got it to work, I wouldn’t worry too much.

@Proniss

It set all of the values to 0 but the contract still appeared to work.

I’m not really sure what you mean by this, but in terms of using:

@Proniss

selfdestruct(owner);

what I was trying to point out is that you would need to change the following line of code in contract Ownable:

// change
address public owner;
// to
address payable public owner;

The address which the selfdestruct function transfers the contract balance to needs to be a payable address. If you use msg.sender, then that is already a payable address by default. However, if you use owner you need to make sure it is either declared as a payable address in contract Ownable (as above), or converted into a payable address within your contract Destructible (as follows):

// either using
address payable receiver = address(uint160(owner));
selfdestruct(receiver);

// or the more concise
selfdestruct(address(uint160(owner)));

As for the adjustment to contract HelloWorld in terms of its inheritance, have a look at this post. I think you’ll find it interesting.

[thecil]

@jon_m

Hi @Proniss

@Proniss

Also it seems Remix doesn’t save my files after I close browser, so unfortunately that code is lost forever.

Oh, that’s annoying for you It must be something to do with your browser settings. My browser saves my Remix files, but I’m not sure how to fix it if it doesn’t. I’m tagging a few colleagues, as one of them may be able to offer some advice @Taha, @thecil, @gabba.

Might be the cookies settings from your browser, which browser are you using?

Maybe you should check on cache settings too, also you can try the remix with another browser, start discarding possible errors.

Hope you find this useful.
If you have any more questions, please let us know so we can help you!

Carlos Z.

[m8rix]

Destroyable.sol

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

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

HelloWorld.sol

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

contract HelloWorld is Ownable, Destroyable{
...
   function withdrawAll() public onlyOwner returns(uint) {
       uint toTransfer = balance;
       balance = 0;
       msg.sender.transfer(toTransfer);
       destroy();
       return toTransfer;
   }

Destroy contract after withdrawlAll

[Tordek]

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

contract Destroyable is Ownable {

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

import "./HelloWorld14_Ownable.sol";
import "./HelloWorld14_Destroyable.sol";
pragma solidity 0.5.12;
    
contract HelloWorld is Ownable, Destroyable {

[jon_m]

Hi @m8rix,

As well as destroying the contract, selfdestruct() also transfers the contract balance to the address placed between the parentheses as the argument. In our case, as msg.sender can only be the contract owner, the funds can only be transferred to the contract owner on destruction of the contract.

As HelloWorld inherits Destroyable and function destroy() is marked public, destroy() will be automatically available within HelloWorld and can be called by the owner from an external service, without having to include an additional function call within HelloWorld.

For these reasons, you can omit the whole of your withdrawAll() function from HelloWorld, as it is only duplicating functionality which already exists without it.

Also, take a look at this post — it explains how you can make an additional adjustment to HelloWorld in terms of the inheritance. I think you’ll find it interesting

[jon_m]

Hi @Tordek,

Nice solution

I’d probably use a name for the function selfDestruct() that differs more from the name of its local function selfdestruct() than by just a capital ‘D’ e.g. destroy() , transferDestroy() etc. This would help to avoid any confusion during further development.

Also, take a look at this post — it explains how you can make an additional adjustment to HelloWorld in terms of the inheritance. I think you’ll find it interesting

[Proniss]

Thanks I’m using Brave, i managed to finish up the course with few issues. Thanks tho that does help for when I mess around in the future.

[marc.berchtold]

Ownable.sol

contract Ownable {

address payable public owner;

modifier onlyOwner() {
    require(msg.sender == owner, "Caller must be contract owner");
    _; //Continue execution
}

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

}

Destroyable.sol

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

HelloWorls.sol

contract HelloWorld is Destroyable{ …

[jon_m]

Hi @marc.berchtold,

Nice work

Could you also add in the top lines for each so that we can see the correct file imports? The contracts won’t execute without these, and so are an important part of the solution.

[marc.berchtold]

Hi @jon_m,

Thank you for the review. Your right, it’s better to add the imports to, especially in this example with inheritance.

pragma solidity 0.5.12;

contract Ownable {
    
    address payable public owner;
    
    modifier onlyOwner() {
        require(msg.sender == owner, "Caller must be contract owner");
        _; //Continue execution
    }
    
    constructor() public {
        owner = msg.sender;
    }
    
}

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

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

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

contract TestContract is Destroyable{ ....

[RLion]

my Destroyable.sol solution :

import “./Ownable.sol”; // Now possible to inherent Ownable module
pragma solidity 0.5.12;
// cursus smartcontracts
// 11 sept 2020
//

// must import Ownable.sol to be able to destroy contract ONLY by Owner
// Function close must be called by owner to destroy the contract

contract Destroyable is Ownable {
    function close() public onlyOwner {     //  onlyOwner is custom modifier
    selfdestruct( address(uint160(owner)) );// `owner` is the owners address MUST be a payable address so conversion needed
    }
}

My Helloworld.sol total file (sorry for all the comments in the source …

import “./Ownable.sol”; // Now possible to inherent Ownable module
import “./Destroyable.sol”;

pragma solidity 0.5.12;
// cursus smartcontracts
// 3 sept 2020
//
// pragma experimental ABIEncoderV2 ; // if you must return a struct as function return

contract HelloWorld is Ownable, Destroyable {
struct Person {
string name ;
uint age;
uint height;
bool senior;
}

event personCreated(string name, bool senior); // memory after string not needed for events ..
event personDeleted(string name, bool senior, address deletedBy);


// constructor alway runs once
uint public balance ; // state variable to hold current balance wich we receive as payment


modifier costs(uint cost){
    require(msg.value >= cost); // check if received payment >= cost defined
    _; // at this place the function itself is done where cost is used as modifier 
}



//mapping( address => Person) public people  ;
mapping( address => Person) private people  ; // change public to private so no getter function is automatic created 
                                              // and ONLY the creater can access the get function  
// now create a array of creators, as admin you do not know the addresses who had used this contract
address[] private creators ;


function createPerson(string memory name,uint age,uint height) public payable costs(1 ether){
    require(age <=150, "Age must be below 150") ;
   
   //check if payment is >= 1 ETH for example
  //  require(msg.value >= 1 ether); // Now it in changed to a modifier in the header
    balance += msg.value ; // add payment to balance in wei !! 18 zeros extra
    
   // This creats a person
   Person memory newPerson;
   newPerson.name   = name ;
   newPerson.age    = age;
   newPerson.height = height;
   if (age > 65){
       newPerson.senior = true ;
   }
   else{
       newPerson.senior = false;
   }
   
   insertPerson(newPerson) ;
   creators.push(msg.sender); // Add sender address to the array
   // Here we add a assert invariant test : 
   // encodePacked = convert a structure to a hex number
   // keccak256    = Hash the hex number
   assert( keccak256(abi.encodePacked(people[msg.sender].name,
                                      people[msg.sender].age,
                                      people[msg.sender].height,
                                      people[msg.sender].senior
                             )
            )
            ==
           keccak256(abi.encodePacked(newPerson.name,
                                      newPerson.age,
                                      newPerson.height,
                                      newPerson.senior
                             )
            )
        );
            
         emit personCreated(newPerson.name,newPerson.senior); // trigger the event   
            
}

function insertPerson(Person memory newPerson) private {
   address creator = msg.sender ; // Now the address of the caller of this function is used        
   people[creator] = newPerson;
}

function getPerson() public view returns(string memory name, uint age,uint height,bool senior){
   address creator = msg.sender; // Used for Mapping Id , which is now the caller adress
   return (people[creator].name,
           people[creator].age,
           people[creator].height,
           people[creator].senior)  ;
}

function deletePerson(address creator) public onlyOwner {
  //  require(msg.sender == owner); // now using onlyOwner as modifier
  
  // make temporary variabel just for the emit, at that point the record is already deleted ...    
    string memory name   = people[creator].name; // just for the emit function needed
    bool          senior = people[creator].senior ; // just for the emit function needed
    
    
    delete people[creator];
    assert(people[creator].age == 0) ; // assert  when age <> 0 (=false), after delete age schould be clear and 0 !!
                                       // when assert triggers it will consume all the GAS remaining !! 
    
    emit personDeleted(name,senior,msg.sender) ; // trigger event
}

function getCreator(uint index) public view onlyOwner returns(address){   // view means not modifying state (function)
   // require(msg.sender == owner,"Caller needs to be the owner"); // Only the admin may use this function  
    return creators[index];
}

function DestroyContract() public {
    close() ;
}

// function to let only the OWNER 
function withdrawAll() public onlyOwner returns(uint){
    uint toTransfere = balance ;
    // VERY IMPORTANT TO RESET BALANCE HERE !!!!  Take the security cource
    balance = 0 ; // need to update state variable FIRST !! Before transfere (safety / security issues)
    
    //1e method
    //msg.sender.transfer(toTransfere); // REVERT when failed, balance will be restored then !!
    //return toTransfere ; // return the actual withdraw amount
    
    
    //2e method
    if (msg.sender.send(toTransfere)){ // return false when failed ==> would NOT REVERT!!
        // success
        return toTransfere ;
    }
    else{
        // failed        
        balance = toTransfere ; // revert balance back to before the failed transaction
        return 0 ; 
    }
    
}

}

[jon_m]

Hi @RLion,

Nicely coded Destroyable.sol file and Destroyable contract

Just a couple of observations…
You can remove the DestroyContract function from your HelloWorld contract because this is duplication:

@RLion

function DestroyContract() public {
    close() ;
}

The close() function containing selfdestruct() is automatically available in HelloWorld because HelloWorld inherits Destroyable.

Also, take a look at this post — it explains how you can make an additional adjustment to HelloWorld in terms of the inheritance. I think you’ll find it interesting