Inheritance Assignment

jon_m · January 30, 2022, 3:55am

Your code meets the assignment objectives. Your inheritance structure is well coded, but we can make it more streamlined by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

Some additional comments …

(1) In your destroy() function, a concise alternative to using the additional receiver variable is to just call selfdestruct() with msg.sender directly…

selfdestruct(msg.sender);

You are using Solidity v0.7.5, and prior to v0.8 msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address e.g. as the payable address argument in the selfdestruct() function call, or as the payable address the transfer method is called on. However, from Solidity v.0.8 msg.sender is non-payable by default, which would mean having to explicity convert msg.sender to a payable address when necessary e.g.

selfdestruct(payable(msg.sender));

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

(2) You’ve modified the withdraw function with the onlyOwner modifier, but this means that only the contract owner will be able to withdraw funds while the Bank contract is deployed and operating normally. The contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so, while the contract is still deployed, it only seems fair that all users should be able to withdraw their funds as well, don’t you think? Your withdraw() function header didn’t include the onlyOwner modifier before. I think during the course we were adding and removing onlyOwner from various functions to demonstrate different scenarios, and I think it might have ended up being left in the withdraw() function in the solution code for this assignment by mistake!

Don’t forget to post your solution to the Events Assignment here, and your answers to the Inheritance Reading Assignment here. The Events Assignment is from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course. I think you may have missed it out because you are missing a Transfer event in your code, and a corresponding emit statement in the transfer() function. The Inheritance Reading Assignment is at the beginning of the Inheritance & External Contracts section of the course, and in the article you will learn, amongst other things, about the multi-level inheritance structure I’ve mentioned above, as well as other types of inheritance structures.

Let me know if you have any questions.

jon_m · December 24, 2021, 4:34am

Hi @jsilver,

Your inheritance structure is well-coded, and your solution meets the assignment objectives

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.

It would be good to also see your full Bank contract, because you’re using a different version of Solidity (v0.8) to the one used in the course (v0.7). This is perfectly fine, and is to be encouraged, but as this is the first code you’ve posted for this course, I think it would be helpful to check that you’ve made the necessary change to the syntax for v0.8. There is, actually, only one change that affects this particular contract, but as you haven’t posted your solutions to the earlier Events Assignment and Transfer Assignment, I can then check your Bank contract contains the correct code for these as well.

If you would like to, you can also post your solutions to these assignmnets separately, in the following discussion topics:

Events Assignment https://studygroup.moralis.io/t/events-assignment/28040?u=jon_m
This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course.

Transfer Assignment https://studygroup.moralis.io/t/transfer-assignment/27368?u=jon_m
This is the assignment from the Payable Functions section of the course, where you have the task of completing the withdraw() function.

Data Location Assignment https://studygroup.moralis.io/t/data-location-assignment/27990?u=jon_m
This assignment doesn’t involve the Bank contract. It’s the first assignment of the course, just before the Events Assignment, near the beginning of the Additional Solidity Concepts section, and you haven’t posted your solution for this one either.

Also, please format your code before posting it. This will make it clearer and easier to read. It will also make 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

Let me know if you have any questions

VBence99 · December 29, 2021, 6:43pm

Destroyable.sol

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable{
    
    function Destruction() public onlyOwner{
        address payable reciever = msg.sender;
        selfdestruct(reciever);
    }
}

Bank.sol

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

contract Bank is Ownable, Destroyable{
...

Ownable.sol

pragma solidity 0.7.5;

contract Ownable {
    address payable public owner;

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

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

jon_m · December 31, 2021, 3:20pm

Hi @VBence99,

Your solution is almost there… but, as a result of a compiler error in Ownable, your Bank contract cannot be deployed. Your inheritance structure is well coded, and once you correct the error, everything should work as it should.

Your modifier is missing _;
This will have generated a compiler error with an error message indicating exactly what the problem is.

A couple of other comments …

(1) We can make the inheritance struture more streamlined by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

(2) In your Destruction() function, a concise alternative to using the additional receiver variable is to just call selfdestruct() with msg.sender directly…

selfdestruct(msg.sender);

You are using Solidity v0.7.5, and prior to v0.8 msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address e.g. as the payable address argument in the selfdestruct() function call, or as the payable address the transfer modifier is called on. However, from Solidity v.0.8 msg.sender is non-payable by default, which would mean having to explicity convert msg.sender to a payable address when necessary e.g.

selfdestruct(payable(msg.sender));

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

(3) You have defined the owner address state variable in Ownable as payable. But this is unnecessary, because you don’t use owner anywhere in your code where it needs to reference a payable address. It would need to be marked payable, however, if you used owner instead of msg.sender (or instead of receiver) to call selfdestruct() with the contract owner’s address, because selfdestruct() takes a single payable address argument…

selfdestruct(owner);

However, because you would only be using owner to reference a payable address once, you could just explicity convert owner to a payable address directly within the selfdestruct() function call, leaving the owner state variable itself defined as a non-payable address…

selfdestruct(payable(owner));

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

codingluan · January 2, 2022, 7:26am

Hi, this is my solution:

These three contract files are in the same directory.

ownable.sol

selfdestruct function will send all of the fund of the contract to the owner address so, to receive the fund, the address has to be payable.

// SPDX-License-Identifier: MIT

pragma solidity 0.7.5;

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

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

destroyable.sol

I find that private and internal functions can’t be payable (but I don’t know why) so I set the close() function with the visibility public.

// SPDX-License-Identifier: MIT

pragma solidity 0.7.5;

import "./ownable.sol";

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

bank.sol

// SPDX-License-Identifier: MIT

pragma solidity 0.7.5;

import "./destroyable.sol";

contract Bank is Destroyable {
// state variables and functions code of the Bank contract goes here
}

jon_m · January 4, 2022, 3:50am

Hi @codingluan,

This is a very well-presented solution which meets all of the assignment objectives

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.

It’s because functions should only be marked payable when they need to receive ether and add it to the contract balance. This will only happen when ether is sent to the contract address from an external address e.g. from Remix, a Web3 client (such as the front-end interface of a dapp), or from an external (i.e. non-derived) contract.

private functions can only be called from within the same contract. A contract is deployed at a single Ethereum address and so only has one contract ether balance. The net effect of transferring ether from a contract balance to the same contract balance would be zero, and therefore pointless.

A parent contract’s internal functions can only be called from within the same parent contract or from within its derived contract(s). A derived contract’s compiled bytecode includes the code it inherits from its parent contract(s), which includes any internal functions. This combined bytecode is then deployed as a single contract, at a single Ethereum address, with a single ether balance. Therefore, any ether received by an internal function, could only have been sent from the same contract address balance as the one now receiving it; and so, as with private functions, this would be pointless, because the net effect would also be zero.

This is correct… and the close() function shouldn’t be marked payable anyway, because it doesn’t need to receive ether. It only needs to send any ether remaining in the contract when it’s destroyed to the contract owner’s external address.

The close() function should have either public or external visibility, so that it can be inherited and called externally by the contract owner’s external address if and when they want to destroy the contract. If it is marked private, then it won’t be inherited. If it is marked internal, then it will be inherited, but only available to call from within the deployed contract. This would require adding an additional public or external function which could then be called externally by the contract owner, and which would contain a function call to close(). But this would defeat one of the main reasons for using inheritance in the first place, namely, to re-use code and avoid code duplication.

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

codingluan · January 5, 2022, 4:16am

Thank you for your informative and clear explanation which helps all these things make sense to me now!

frandolz · January 5, 2022, 2:30pm

pragma solidity 0.8.11;

contract Ownable {
address internal owner;

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

constructor(){
    owner = msg.sender;
}

}

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

contract Destroyable is Ownable {

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

And for the header of Bank contract

pragma solidity 0.8.11;
import “./Ownable.sol”;
import “./Destroyable.sol”;

contract Bank is Ownable, Destroyable{}

jon_m · January 5, 2022, 6:15pm

Hi @frandolz

Your solution is almost there… but, you should find that your Bank contract cannot be deployed as a result of a compiler error in Destroyable. Once you correct this error (see below) everything should work as it should.

Your inheritance structure is well coded, but we can make it more streamlined by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

You have chosen to use the more up-to-date Solidity v.0.8, which is only to be encouraged However, you need to pay close attention to any errors that are generated by the compiler where you wouldn’t expect this based on the syntax taught in this course (Solidity v0.7). Although, in actual fact, the only difference in syntax between these two versions which affects the code covered in this course concerns msg.sender.

Prior to Solidity v0.8 msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address. That’s why the model solution for this assignment (based on Solidity v0.7 syntax) allows msg.sender to be assigned to a payable address variable (receiver) without having to explicitly convert it to a payable address …

However, from Solidity v0.8, msg.sender is non-payable by default, which means having to explicity convert it to a payable address when necessary. And this is why you need to make the following modification to your code in order for it to compile …

address payable receiver = payable(msg.sender);
selfdestruct(receiver);

Once you’ve corrected this, it would be better to also see your complete Bank contract to check that everything there is also coded correctly, because using Solidity v0.8 requires the same modification to be made in one specific place in this contract as well. The compiler should indicate where this is with a red error marker, and the error message should confirm that it is the same type of error as the one in your Destroyable contract.

Some additional comments …

In your destroy() function, a concise alternative to using the additional receiver variable is to just call selfdestruct() with msg.sender directly…

selfdestruct(payable(msg.sender));

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

If you’d like to fully complete this course, don’t forget to also post your solution to the Events Assignment here, and your answers to the Inheritance Reading Assignment here. The Events Assignment is from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course. The Inheritance Reading Assignment is at the beginning of the Inheritance & External Contracts section of the course, and in the article you will learn, amongst other things, about the multi-level inheritance structure I’ve mentioned above, as well as other types of inheritance structures.

Just let me know if you have any questions

thanasornsawan · January 6, 2022, 8:38am

Bank file

pragma solidity 0.7.5;

import "./Ownable.sol";

import "./Destruct.sol";

contract BankMap is Ownable,SelfDestruct {
. . .
}

Ownable file

pragma solidity 0.7.5;

contract Ownable {

    address public owner;

    //modifier use as small function that will run first before main function

    //use case of modifier is when many functions need to use the same logic

    modifier onlyOwner {

        //check the person who can add balance is only owner do

        require(msg.sender == owner, "only the owner can add money/withdraw");

        _; //run the function

    }

    constructor() {

        //msg.sender is the address who deploy contract

        owner = msg.sender;

    }

}

Destroy file

pragma solidity 0.7.5;

import "./ownable.sol";

contract SelfDestruct is Ownable {

 

     function destroy() public onlyOwner{

       selfdestruct (owner);

       }

   

}

jon_m · January 8, 2022, 3:08am

Hi @thanasornsawan,

Your solution is nearly there… but your Bank contract cannot be deployed as a result of a couple of compiler errors: one in SelfDestruct, and another in either BankMap or SelfDestruct. Once you correct these errors (see below) everything should work as it should.

Your inheritance structure is mostly well-coded, but we can make it more streamlined by having BankMap explicitly inherit SelfDestruct only, because it will inherit Ownable implicitly via SelfDestruct. This will give you a multi-level inheritance structure.

Compiler errors

(1) BankMap / SelfDestruct

Your Destruct.sol file imports a file called ownable.sol, but your Bank.sol file imports a file called Ownable.sol. You need to make sure that the name of the actual file containing your Ownable contract is spelt correctly and consistently (in terms of whether it starts with a capital or lower case letter) in both import statements. However, if, as I’ve suggested above, BankMap only explicitly inherits SelfDestruct, you can just remove import "./Ownable.sol"; from Bank.sol completely.

(2) destroy() function in SelfDestruct

If you read the compiler error you should be getting for this line of code, you will see that owner needs to be explicity converted to a payable address. This is because owner is defined in your Ownable contract as a non-payable address …

… but the method selfdestruct requires a payable address argument, so that when it’s called, it will automatically transfer the ether balance remaining in the contract to the address it is called with (in your case, owner).

You can easily fix this is several different ways. Which would you choose? If you’re not sure, have a look at some of the other students’ solutions, and the feedback and comments they’ve received, posted here in this discussion topic. You will find a lot of useful information here.

I would also change your error message in the onlyOwner modifier’s require() statement …

You should no longer be restricting access to any of the BankMap contract’s functions (for the reasons I explained in my feedback for your Transfer Assignment) and so this error message needs to either refer to the restriction this modifier applies to the destroy() function in SelfDestruct, or it needs to be more generic so that it makes sense in all of its possible use cases.

Let me know if you have any questions about any of these points

firequo · January 10, 2022, 12:13am

Ownable:

 pragma solidity 0.7.5;
contract Ownable {
    address owner;
    modifier OnlyOwner {
        require(msg.sender == owner, "Only the owner can call this function");
        _;
    }
    constructor(){
        owner = msg.sender;
    }
}

Destroyable:

pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable{
    function destroy() internal OnlyOwner{
        selfdestruct(payable(owner));
    }
}

Bank:

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

contract Bank is Ownable, Destroyable {

r0bz078 · January 10, 2022, 12:20am

Ownable Contract

pragma solidity 0.7.5;

contract Ownable {
    address payable owner;

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

Destroyable Contract

pragma solidity 0.7.5;

import "./ownable.sol";

contract Destroyable is Ownable {

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

Bank Contract

pragma solidity 0.7.5;

import "./ownable.sol";

import "./destroyable.sol";

contract Bank is Ownable, Destroyable {

InterstellarX · January 10, 2022, 5:43am

Hey, so I am not sure how to automatically send crypto that another person uploaded to the contract…however, my self-destruct contract stops the contract for other users that are not the admin (account that created the contract). Can you help me understand what would allow the contract to automatically send all of the crypto to the address as stated in the instructions? I am stuck on this part but everything else seems to work:

`Ownable.sol`

// SPDX-License-Identifier: None

pragma solidity 0.7.5;

// way to inhert from contract ownable for owner functionality
contract Ownable {

    // mapping(address => uint) public balances;
        // by adding public to state variable (visibility level), automatically creates a function that you can get this variable
    address public owner;

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

    // sets the owner as owner of contract (msg.sender)
    constructor() {
        owner = msg.sender;
    }

}

`Destroyable.sol`

// SPDX-License-Identifier: None

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {

    // 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 onlyOwner { 
        selfdestruct(msg.sender); 
    }
}

`Bank.sol`

// SPDX-License-Identifier: None

pragma solidity 0.7.5;

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

contract Bank is Ownable {

    mapping(address => uint) balance;
    
    // define event
        // indexed = by eth nodes, can use params to search/query for events in the past
    event depositDone(uint amount, address indexed depositedTo);

    event balanceTransfered(uint amount, address indexed transferedFrom, address transferredTo);

    event balanceWithdrawn(address indexed withdrawnFrom);

    
    // for anyone to deposit
        // payable - allows function to receive money when people call it
    function deposit() public payable returns (uint) {
        // if you deposit 1 ether, it should be attributed to account
            // don't need to worry about memory or storage, because saved to the blockchain
            // balance mapping keeps track of internal transfers/sending
        balance[msg.sender] += msg.value; // to internally track who deposited money
        // add event 
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender]; 
    }

    // function for folks to withdraw eth to their address
    function withdraw(uint amount) public onlyOwner returns (uint) {
        // msg.sender is an address 
            // you can define an address payable sender and name whatever name you want to
            // address payable toSend = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4;
        
        // Transfer has revert abilities...if fails, will revert non-gas eth
            // transfer has built in error handling
            // MUST check balance of user to ensure can't overdraw/withdraw other people's money
            // toSend.transfer(amount);
        require(balance[msg.sender] >= amount); // prevents withdraw of others' money
        balance[msg.sender] -= amount; // remove amount from balance before transfer occurs
        msg.sender.transfer(amount); // run the transfer function
        emit balanceWithdrawn(msg.sender);
        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, "Balance not sufficient");
        require(msg.sender != recipient, "Don't transfer money to yourself");

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

       // add event 
       emit balanceTransfered(amount, msg.sender, recipient);

       assert(balance[msg.sender] == previousSenderBalance - amount);
    }

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

    function close() onlyOwner public {
    }
}

thanasornsawan · January 10, 2022, 10:24am

@jon_m thank you for help fix my understand.I just understand that “method selfdestruct requires a payable address argument” and I didn’t check the file name when import well.

jon_m · January 11, 2022, 5:31am

Hi @firequo,

Your solution is nearly there… but the Bank contract cannot be destroyed by the contract owner. The destroy() function is inherited by Bank, but when you deploy your Bank contract, you will see that there is no button available for the owner to call the destroy() function externally and trigger selfdestruct(). This is because you’ve marked the destroy() function with internal visibility, which makes it available to call from within the derived contract (Bank), but it isn’t available to be called externally. So, it needs to have either public or external visibility.

Once you correct this, you will be able call destroy() with the contract owner’s address; and because everything else is well coded, the contract will be destroyed, and the ether remaining in the contract (the contract address balance) will be successfully transferred to the owner’s external address.

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

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

c1cag1b3 · January 11, 2022, 4:39pm

Please help to identify mistake as I cannot Compile non of these contracts.

contract Ownable {

    address public owner;

    modifier onlyOwner {

    require(msg.sender == owner);

    _; //run the function

    }  

    constructor(){

    owner = msg.sender;

    }

}

import "./Ownable.sol";

contract Destroyable is Ownable {

 

 function close() onlyOwner public  {

  selfdestruct(owner);

 }

}

import "./Destroyable.sol";

contract Bank is Destroyable {

jon_m · January 11, 2022, 6:53pm

Nice solution @r0bz078

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.

Let me know if you have any questions

jon_m · January 11, 2022, 7:04pm

Hi @thanasornsawan,

Yes, you’ve understood correctly.

It would be good to see how you correct this, so that your code compiles and selfdestruct (i) destroys the contract and transfers the ether balance remaining in the contract to the address it is called with (in our case, owner).

Let me know if you have any questions

Tomaage · January 11, 2022, 7:51pm

Bank.sol

pragma solidity 0.8.11;

import "./Destroyable.sol";

contract Bank is Destroyable {

    mapping(address => uint) balance;

    event depositDone(uint amount, address depositedTo);

    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 onlyOwner returns (uint) {
        require(balance[msg.sender] >= amount);
        payable(msg.sender).transfer(amount);
        return balance[msg.sender];
    }

Destroyable.sol

pragma solidity 0.8.11;

import "./Ownable.sol";

contract Destroyable is  Ownable {

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

 }

}

Ownable.sol

pragma solidity 0.8.11;

contract Ownable {

    address payable owner;

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

}

Questions:

Is it needed to put the version number into every file, or just the base contract file? The compiler asked for the version number to be added.
Since im using compiler 0.8.11 I needed to put payable in the ownable contract adress and constructor. Is this right? I could not make it work until I added there.