Inheritance Assignment

pragma solidity 0.7.5;

import “./Ownable.sol”;

contract Destroyable is ownable {

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

}

1 Like
import "./ownable.sol";

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

And the main contract which is HelloWorld:

import "./destroyable.sol";

contract Bank is destroyable
1 Like

(1) Ownable

pragma solidity 0.7.5;

contract Ownable {
address public owner;

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

}

(2) Selfdestruct

pragma solidity 0.7.5;

contract Storage {
address payable private owner;
uint256 number;

constructor() {
owner = msg.sender;
}

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

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

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

(3) Bank

pragma solidity 0.7.5;

import"./Ownable.sol";
import"./Selfdestruct.sol";

contract Firstcontract is Ownable, Storage{

mapping(address=>uint)balance;
1 Like

Ownable:

pragma solidity 0.7.5;

contract Ownable{

address public owner; 
modifier onlyOwner {
    require (msg.sender == owner, "Only owner can access this function");
    _; 
}

constructor() {
    owner = msg.sender;
}

}

Destroyable:

pragma solidity 0.7.5;

import “./assignment_Ownable.sol”;

contract Destroyable is Ownable {

function destroy() public onlyOwner {
address payable owner = msg.sender;
selfdestruct(owner);
}

}

1 Like

The basic functionality required in the assignment works, but I have two big problems. Please help!

  1. If I compile using latest compiler (at time of posting it’s 0.8.7) I get some major problems with addressed not being payable. The most problematic is the withdraw function in the Bank contract:

function withdraw(uint amount) public payable returns (uint){
require(balance[msg.sender] >= amount);
balance[msg.sender] -= amount;
msg.sender.transfer(amount); // this line is the problem
return balance[msg.sender];
}

The compiler says:
Type Error: “send” and “transfer” are only available for objects of type “address payable”, not “address”

How can I fix this across the 3 contract files so it will compile properly with 0.8.7?

  1. After calling the self-destruct function the contract does indeed stop functioning, however the deposit function will still pull ether out of the account. Any idea why and how I can fix?
1 Like

why do we need to create a new payable variable name receiver and assign it to msg.sender and then destroy the receiver instead of destroying the owner variable directly.

2 Likes
import "./Ownable.sol";

contract Destroyable is Ownable {
    
    
    function destroy() public onlyOwner {
        address payable owner = msg.sender;
        selfdestruct(owner);
    }
}
1 Like

Okay, I’ve learned a lot with this assignment. I have to say, I don’t think I could have done this very well without looking at some great examples of other people’s contracts in this forum thread, and reading some great descriptions and explanations of why and how they wrote the contracts the ways they did. I learn well that way.

So here is a multi-level inheritance:

Ownable.sol :

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

//\\ -- OWNABLE -- //\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\
contract Ownable {

    address payable owner;
    
    modifier onlyOwner() {
        require(msg.sender == owner, "Caller must be contract owner");
        _; // run the function
    }

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

}

Destroyable.sol :

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

import "./Ownable.sol";

//\\ -- DESTROYABLE -- //\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\
contract Destroyable is Ownable {

    // DESTROY - //////////////////////////////////
    function destroy() public onlyOwner {
        // destroy
        // - requires a payable address
        // - automatically transfers remaining contract balance to an address
        selfdestruct(owner);
    }
}

Bank.sol :


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

// import "@nomiclabs/builder/console.sol";
import "./Destroyable.sol";

//\\ -- BANK -- //\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\//\\
contract Bank is Ownable, Destroyable {

    // address owner;
    mapping (address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    event transferDone(uint amount, address indexed sentFrom, address indexed sentTo);

    modifier enoughBalance() {
        // check if balance of sender is sufficient
        // require(balance[msg.sender] >= amount,"Balance not sufficient");
        _; // run the function
    }

    modifier senderNotRec() {
        // check for redundancy
        // require(msg.sender != toRecipient, "Don't send money to yourself");
        _; // run the function
    }
        
    // INIT //////////////////////////////////
    constructor() {
        owner = msg.sender;
    }
    
    // DEPOSIT - payable //////////////////////////////////
    function deposit() public payable returns(uint) {
        balance[msg.sender] += msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    // GET BALANCE - read only //////////////////////////////////
    function getBalance() public view returns(uint) {
        return balance[msg.sender];
    }
    
    // WITHDRAW //////////////////////////////////
    function withdraw(uint amount) public returns (uint)  {
        // msg.sender is an address, and address has a method to trasfer 
        msg.sender.transfer(amount);
        // adjust balance
        balance[msg.sender] -= amount;
        return balance[msg.sender];
    }
    
    // TRANSFER TO //////////////////////////////////
    function tranferTo(address recipient, uint amount) public {
        // check if balance of sender is sufficient
        require(balance[msg.sender] >= amount,"Balance not sufficient");
        // check for redundancy
        require(msg.sender != recipient, "Don't send money to yourself");

        uint previousSenderBalance = balance[msg.sender];
        _transfer(amount, msg.sender, recipient);

        assert(balance[msg.sender]==previousSenderBalance - amount);
        emit transferDone(amount, msg.sender, recipient);
    }

    // _TRANSFER - private //////////////////////////////////
    function _transfer(uint amount, address from, address to) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
    
}

I’ve some questions about this in my next comment …

1 Like

Questions (about my contract code above):

1 - why in Destroyable can’t I just include the owner state variable in selfdestruct(owner) ?
I saw most everyone using selfdestruct(msg.sender). Why? If owner is already declared and required to be payable msg.sender in Ownable, why not just refer to owner?

2 - in Bank (which is a bit messy code, because I’m playing around with some potential modifiers), since Ownable is being derived through Destroyable, is the constructor required? I just tried it in Remix with and without the constructor, and both ways worked. I guess I would only need a constructor here if I was adding in addition state variable definitions only needed in Bank.

3 - By the way, I am burning to know how Deposit() works. It makes not sense to me.
When I try in Remix, nothing happens. The balance does not change. Is it supposed to? How does deposit transfer value, from where? Why doesn’t the function accept a value? If no value is being input, then what is being deposited? Makes no sense to me, and I’d be so appreciative if someone could explain it to me.

1 Like

I just looked at the solution code, and have another question:

why is owner made internal?

contract Ownable {
    address internal owner;

1 Like

Disclaimer: I’m a newbie like you. Having said that:

I think its clearer to read if its the owner. In the 0.8.6 version there are no implicit conversions from address to address payable. I would think explicitly stating an address is payable improves readaility.

Definitely not required to define the same variable again.

The balance is supposed to change and it does. You do not need specify the amount in an argument of the function call, because the message with which you call the deposit function is supposed to have ETH. That value is stored msg.value. The function can accept value from a message because it’s payable - that is what the attribute means.
You can specify the amount sent in the message in the value tab in remix like so:
image

2 Likes

If its internal it’s only accessible to the contract. I think anyone should be able to query which address deployed the contract, so making it a secret is kind of pointless.

2 Likes

Hey @Attiss, hope you are well.

Take a look at this reply i made in another topic which is the same issue that you are mentioning

Carlos Z

ah, thank you so much. I had not realized that the Value field was associated to the deploy parameters. It’s all making much better sense now,

1 Like

Thanks Carlos! That cleared it up.

1 Like

Hi @omkar_c,

You don’t need to do it that way. It is a more long-winded (or some would say more clearly broken-down step-by-step) version of the following equally valid solution:

selfdestruct(msg.sender);

We can use msg.sender to reference the contract owner’s address, because the onlyOwner modifier restricts access to the destroy() function to the contract owner, and so msg.sender can only ever be the contract owner address anyway.

In Solidity v0.7, msg.sender is a payable address type by default, and so there is no need to explicitly convert it whenever we need it to be payable, e.g. in selfdestruct() . However, in Solidity v0.8, msg.sender is now non-payable by default, but instead of converting it to a payable address via a separate variable, we can perform the conversion within the selfdestruct() function call itself…

selfdestruct(payable(msg.sender));

Alternatively, you can reference the owner variable by either:

(1) Declaring the owner state variable in Ownable with a payable address type, instead of with the default, non-payable address type; or

(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 destroy() public onlyOwner {
    selfdestruct(payable(owner));
}

Have a look at this post for further details about the use of a separate receiver variable in the model solution.


And just to clarify… we are not destroying the contract owner’s address. We’re destroying the smart contract. The owner’s address is passed to selfdestruct() because, as well as destroying the contract, calling selfdestruct() will transfer the contract’s remaining ether balance to its mandatory payable address parameter. To receive ether an address must be payable.

Let me know if anything is unclear, or if you have any further questions.

Hi @simfin94,

Your Destroyable contract is looking good :ok_hand: but can we also see your code for the other two contracts in your inheritance structure?

We especially need to see what modifications you’ve made to the start of your Bank.sol file, and Bank contract header, for the inheritance. This is an important part of the solution, because the aim of this assignment is for our Bank contract to inherit both the selfdestruct() and contract ownership functionalities, so that both are available when we just deploy Bank i.e. we should be able to deploy Bank, perform some transactions, then destroy the Bank contract, transferring any remaining ether balance to the caller of the selfdestruct function (here, the contract owner).

It would also help to see your Ownable contract, because you’ve used the same name owner for your local payable address variable (in the destroy function in Destroyable) as the state variable in Ownable — unless of course you’ve changed the name of the state variable… Even if they do have the same name, your solution still works, but if you’re going to assign msg.sender (as the owner address) to a separate variable before passing it to selfdestruct(), then I think it would be much clearer to give this variable a different name.

Alternatively, you could use one of the other methods for passing the contract owner address to selfdestruct() , which I’ve outlined in this post.

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

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

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

pragma solidity 0.7.5;

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


pragma solidity 0.7.5;

import "./Destroyable.sol";
import "./Ownable.sol";
contract Bank is Ownable,Destroyable{
   mapping(address => uint) balances;
   
  function withdraw(uint amount) public {
     
      owner.transfer(amount);
      balances[owner] += amount;
      close();
  }
 
}

I don’t see Destroyable 's destroy function is called in Bank contract.
If destroy is not called, then why you defined the destroy function in parent contract?

ownable.sol

contract Ownable {

address owner;

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

constructor() {
    owner = msg.sender;
}

}

destroyable.sol

import “./ownable.sol”;

contract Destroyable is Ownable {

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

}

bank.sol

pragma solidity 0.7.5;

import “./ownable.sol”;
import “./destroyable.sol”;

contract Bank is Ownable, Destroyable {


1 Like