Inheritance Assignment

[jon_m]

Nice solution @Meriem

Can we also see what modifications you’ve made to the start of your Bank.sol file, and Bank contract header, for the inheritance?

You can also develop the inheritance structure one stage further, in order to keep the contract ownership and contract destruction functionality separate — each within its own contract e.g. Ownable and Destroyable. You would then need to decide how to change your exisiting code for this inheritance structure, because you would now have three contracts instead of just two.

@Meriem

… there is something a little bit confusing to me. Why does it have to be ownable if the address in the function is msg.sender anyway?

I’m not sure I fully understand your question. Are you asking why we need the “ownable” functionality (as follows) at all here?

@Meriem

address owner;

constructor(){
    owner = msg.sender;
}

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

If we don’t restrict close() to the contract owner (the deployer of the derived contract, Bank) then anybody could be msg.sender, call close() and destroy Bank by triggering selfdestruct. Furthermore, when selfdestruct() is triggered, the ether balance remaining in Bank will be transferred to whichever address argument it is called with. It, obviously, makes sense that we would want to restrict this to an address such as the contract owner’s.

So, by making close() onlyOwner , the msg.sender can only be the owner address, meaning that only the owner can destroy their own contract. In Solidity v0.7.5, msg.sender is payable by default, so instead of:

@Meriem

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

we can have the more concise:

selfdestruct(msg.sender);

We could also call selfdestruct() with owner , like this…

selfdestruct(payable(owner));

or, by storing the owner address as a payable address in the state variable, from the outset…

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

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

[DylanHepworth]

Destroyable Contract

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {

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

Bank Contract Imports

pragma solidity 0.7.5;

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

contract Bank is Ownable, Destroyable { . . . 

Ownable Contract

pragma solidity 0.7.5;

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

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

[jon_m]

Hi @Bryan,

You’ve got the right idea in terms of the multi-level inheritance structure. I think the following is probably just a slip…

@Bryan

import "./Ownable.sol";

contract Bank is Destroyable {...}

… you should be importing Destroyable.sol (not Ownable.sol).

And here, you don’t need the parentheses after the inherited contract name…

@Bryan

contract Destroyable is Ownable(){...}

You’re also missing the constructor in Ownable, but maybe you’ve just omitted that to keep the post shorter…

However, the main issue with your solution, and which should be giving you a compiler error, is…

@Bryan

selfdestruct(owner);

The address passed to selfdestruct() needs to be a payable address, but yours isn’t. This is so that on selfdestruct any remaining ether in the contract can be paid/transferred to the owner. Can you work out how to fix this? There are a couple of different ways it can be coded …

[jon_m]

Hi @Michael_Gerst,

Can we see your other two contracts which are also part of this inheritance structure?

Your Destroyable contract relies on you having made an additional change in Ownable. That’s why we need to see it as part of your solution. The address passed to selfdestruct() determines where any remaining ether balance is transferred to, but does not restrict who can call this function. At the moment, anyone can call your destroyContract function and trigger selfdestruct(). The idea is to restrict this to the contract owner, for obvious reasons

Apart from this, your Destroyable contract is looking good

We also need to see what modifications you’ve made to the start of your Bank.sol file, and Bank contract header, for the inheritance.

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

[PhilD99]

Thanks jon_m

Here’s the additional code at the start of the BankDeposit contract:

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

contract HelloWorld is Ownable, Destroyable {

import “Ownable.sol”;

contract BankDeposit is Ownable, Destroyable {

[PhilD99]

Oops, should be:
import"Ownable.sol";
import"Destroyable.sol";

contract BankDeposit is Ownable, Destroyable {

[jon_m]

Great solution @santoretech
… and your code in general is looking great

You have made sure your owner address in Ownable is payable , which is what it needs to be so that selfdestruct() is called with a payable address and any remaining ether in the contract can be paid/transferred to the owner

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

Unrelated to this particular assignmnet, but worth mentioning…
… you’ve included an additional, well-coded, and really useful, modifier:

@santoretech

modifier enoughBalance (uint amount) {
    require(balance[msg.sender]>= amount, "Not enough funds");
    _; //Run the function
    }

… but then you haven’t implemented it in the two functions where it can be used instead of a full require statement in the function body, to avoid code duplication.

You’re making great progress! Keep it up

[jon_m]

Hi @dani88,

@dani88

The error message is on line 23, the deposit function. The message states ParserError: expected type name function deposit(1ether) public payable costs(1 ether) {
}

The error message is telling you that a function parameter should be written using the syntax:

(dataType name)
// e.g.
(uint payment)

(1 ether)  is not a valid input. The value of 1 ether being sent to the function can be automatically referenced in the function as msg.value , by default, just like you are referencing the address calling the function as msg.sender. And just like msg.sender, msg.value does not need to be explicitly passed as a function parameter.

You have correctly included the modifier costs(1 ether).
The value you input into the Value field in Remix (below the dropdown with the account addresses and Gas Limit field) when you call deposit(), is the msg.value in your modifer’s require statement…

@dani88

modifier costs (uint cost){
    require(msg.value >= cost);
    _;
}

… and so the value sent to the function (e.g. 1 ether) will be compared with the modifier’s cost parameter (fixed at 1 ether for the deposit function) to check that it is equal to, or greater than 1 ether.

Once you’ve corrected that, your code as it is will still throw a lot more compiler errors, so I assume this is still a work in progress, right?

It’s fantastic that you’re working on creating your own derived contract, but it might be easier to work on that once you’ve completed this assignment using the HelloWorld contract, which, apart from the code you need to add for the inheritance, should already be compiled, deployable and workable.

@dani88

function close() public onlyOwner returns() {
    selfdestruct(owner)   // you are missing a semi colon here!
    msg.sender.transfer(balance);
    return toTransfer;        
}

When the selfdestruct() function is called it will automatically transfer the ether balance remaining in the contract to the address it is called with (in your case, owner). For this to happen, this address needs to be a payable address. There is no need to add any additional code to perform this transfer, and your close() function does not need to return a value. There won’t be any data to return anyway, because once selfdestruct() is executed, the contract (and its data stored in the contract state) will cease to exist!

Also, the idea is to put the contract destruction functionality in a separate contract called Destroyable (like we did with the ownership functionality and Ownable). You will then have 3 separate contracts, and the inheritance structure you code should be such that when just your derived contract (e.g. HelloWorld) is deployed, it will inherit the functionality of both Destroyable and Ownable.

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

[jon_m]

Nice solution @DylanHepworth

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

[Michael_Gerst]

Hey Jon,

Here is Ownable

pragma solidity 0.7.5;

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

and Bank

pragma solidity 0.7.5;

import "./ownable.sol";

interface GovernmentInterface{
    function addTransaction(address _from, address _to, uint _amount) external;
}
contract Bank is Ownable{
   
   
    GovernmentInterface GovernmentInstance = GovernmentInterface(0xC2F2b73709026aD4A31b22C95689E47ae8423913);
   
    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 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);
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        GovernmentInstance.addTransaction(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        
        balance[from] -= amount;
        balance[to] += amount;
    }

}

Thanks again for helping

[Bryan]

Thanks Jon, here is an updated Ownable contract. The owner variable is now payable and a constructor is included.

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

[nutrina]

Ownable.sol:

pragma solidity >=0.7.0 <0.9.0;

contract Ownable {
    
    address public owner;
    
    modifier onlyOwner {
        require(msg.sender == owner, "You need to be owner");
        _; // run the function
    }
    
    constructor() {
        owner = msg.sender;
    }
}

SelfDestroyable.sol:

pragma solidity >=0.7.0 <0.9.0;

import "./Ownable.sol";

contract SelfDestoryable is Ownable {

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

And our Bank contract:

// SPDX-License-Identifier: GPL-3.0

pragma solidity >=0.7.0 <0.9.0;


import "./SelfDestroyable.sol";
    
contract Bank is SelfDestoryable {
    
    mapping(address => uint) internal balance;
    
    event depositDone(uint amount, address indexed 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 returns(uint) {
        require(amount <= balance[msg.sender], "Insufficient balance");
        uint initialBalance = balance[msg.sender];
        
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        
        assert(balance[msg.sender] == initialBalance - 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, "Insuficient balance!");
        // require(msg.sender != recipient, "Don't transfer money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    } 
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    } 
}

[jon_m]

Hi @Michael_Gerst,

Based on your Ownable contract, you should be getting compiler errors for these lines in the Destroyable contract you posted previously:

@Michael_Gerst

contract Destroyable is ownable{

Ownable will not be inherited unless the contract name is written exactly the same — including the initial capital letter.

@Michael_Gerst

selfdestruct(owner);

The compiler should throw an error for this, once you correct the first one. You can see in the error message, that owner needs to be a payable address. The address passed as an argument to the selfdestruct function must be payable because this is the address that any ether remaining in the contract will be transferred to on destruction. At the moment, owner is inherited from Ownable, where it is defined as a non-payable address, here…

@Michael_Gerst

address public owner;

Do you know how to make this address payable? There are a couple of alternatives.

Do you know how to compile your code to check for these kinds of errors? If not, let us know, so we can help you

Also, don’t forget…

@jon_m

The address passed to selfdestruct() determines where any remaining ether balance is transferred to, but does not restrict who can call this function. At the moment, anyone can call your destroyContract function and trigger selfdestruct(). The idea is to restrict this to the contract owner, for obvious reasons

Destroyable will inherit the onlyOwner modifier from Ownable… so use it to restrict access to destroyContract()

Bank

The aim of this assignment is for our Bank contract to inherit the selfdestruct() and Ownable functionality, 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).

At the moment, we can’t do this, because your Bank contract only inherits Ownable.

To complete each coding assignment, and before moving on to the next lesson, you need to make sure (i) your code compiles without any errors (ii) your contract (in this case the child contract Bank) deploys successfully; and (iii) your deployed contract is able to perform all of the necessary transactions i.e. it does what you expect it to. If it doesn’t do all of these things, then you can have a look at other students’ solutions, and the feedback and suggestions they’ve received, posted in these assignment discussion topics. This is where you can also post any questions you have about the assignment, and ask for help

[jon_m]

Great solution @nutrina

Your use of a multi-level inheritance structure is the most appropriate here. You are correct that Bank only needs to “directly” inherit Destroyable, because it will then “indirectly” inherit Ownable via Destroyable.

Just one comment… by using…

@nutrina

pragma solidity >=0.7.0 <0.9.0;

… the Solidity compiler in Remix automatically defaults to the latest version (v0.8.4). But your code will only compile successfully using v0.7, because from v0.8 msg.sender is a non-payable address by default. Previously it was payable by default. So, you either need to adjust the version range to include only those which are compatible with you code, or find those instances of msg.sender which need to be converted to payable addresses, and make the relevant adjustments. I would be interested to see if you can work out how to do this yourself

You’re making great progress! Keep it up!

[guibvieira]

pragma solidity 0.7.5;

contract Destroyable is Ownable{
    function close() public onlyOwner { //onlyOwner is custom modifier
      selfdestruct(owner);  // `owner` is the owners address
    }
}

[dani88]

stuck:
I tried to make it super simple, then searched the internet but it didn’t work. What am I missing?

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

contract leaseAgreement is Ownable, Destroyable{
    
    function close() public onlyOwner returns(){
        selfdestruct(owner);
        
    }
    
}
```![Screen Shot 2021-04-29 at 1.48.40 PM|690x277](upload://kqqFioFr1rNBpGIj8ve7ItEsv6B.png)

[Meriem]

Thanks jon_m.
I had to go back to the previous lectures to better understand the matter but i get it now.

[Meriem]

Im sorry to ask so many questions but after testing the destroyable contract I noticed that if other addresses deposit fund in the bank contract and then the owner triggers the destroy function all their funds will be sent to him, right?
Then how can users trust a dapp with this capability?

[Meriem]

pragma solidity 0.7.5;

contract Ownable {

address owner;

constructor(){
    owner = msg.sender;
}

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

}

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

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

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

contract Bank is Ownable, Destroyable {

mapping (address => uint) balance;

event depositDone(uint amount, address indexed depositedTo);

function deposit() public payable{
    balance[msg.sender] = msg.value;
    emit depositDone(msg.value, msg.sender);
}

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

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

}

[jon_m]

Hi @guibvieira,

Can we see your other two contracts which are also part of this inheritance structure?

@guibvieira

pragma solidity 0.7.5;

contract Destroyable is Ownable{ ... }

Don’t forget to also import the file which contains Ownable.

Passing owner instead of msg.sender to selfdestruct() relies on you having made an additional change in Ownable. That’s why we also need to see Ownable as part of your solution.

Apart from this, your Destroyable contract is looking good

We also need to see what modifications you’ve made to the start of your Bank.sol file, and Bank contract header, for the inheritance.

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