Inheritance Assignment

decrypter · July 14, 2021, 5:32pm

Ok, got it, even if close() is public it cant be called from the outside if it is overridden in a derived contract. My understanding is there is no way to get around this, for example by creating and casting a function pointer to Destroyable.close();

Even if close() in Destroyable has public visibility, it can’t just be called directly from outside Bank.

Just for clarification, if close() is public and overridden only the overridden function can be called from the outside. However if close() is public and not overridden it can indeed be called from outside the contract.

jon_m · July 14, 2021, 9:11pm

Yes that is correct. I think we now both understand each other

Thank you for such a stimulating discussion. You really got me thinking

Here is some further evidence for you… to back up my assertion…
The discussions I’m linking to predominantly deal with overridden inherited functions which have external visibility. But as the conclusion is that such a function would be impossible to call, but possible to call internally if public, then I think it is safe to draw the same conclusion that I have about overridden inherited functions marked public. Also, in the 2nd discussion, the post on 28 January 2020 seems to me to be a definitive answer confirming that the EVM doesn’t allow an overridden inherited function to be called externally (from outside the deployed derived contract where the overriding function is located).

https://ethereum.stackexchange.com/questions/90243/calling-super-class-external-functions
https://github.com/ethereum/solidity/issues/3881

In these discussions the syntax super is referred to. In case you don’t know, in our example super.close() would be an equally valid alternative to Destroyable.close()

Jackthelad · July 18, 2021, 4:03am

Ownable.sol

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

Terminate.sol

pragma solidity 0.7.5;
import "./Ownable.sol";
contract Terminate is Ownable {
    function terminate() public onlyOwner {
    selfdestruct(payable(msg.sender));
    }
}

BankofJ.sol header

pragma solidity 0.7.5;
import "./Terminate.sol";
contract Bank is Terminate{

Cero · July 15, 2021, 8:57am

Ok, it makes sense for Destroyable contract, but where is the point to have for example onlyOwner in withdrawal function in helloworld contract? Do we need onlyOwner there in some cases?

gregD · July 17, 2021, 4:13pm

bank.sol

pragma solidity 0.7.5;

import "./destroyable.sol";

contract Bank is Destroyable {
    
    mapping(address => uint) 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(balance[msg.sender] >= amount, "Insufficient funds");
        
        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, "Insufficient balance");
        
        require(msg.sender != recipient, "Cannot transfer 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;
    }
}

destroyable.sol

pragma solidity 0.7.5;

import "./ownable.sol";

contract Destroyable is Ownable{
    
    function closeContract () onlyOwner public {
        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);
        _;
    }
}

When testing the bank contract, all the functions seem to work except when the owner destroys the contract the funds are not returned to owner, the balance afterwards is zero

I am struggling to solve this, any help would be amazing

Andre_Mark · July 17, 2021, 7:56pm

Ownable

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

Destroyable

import "./Ownable.sol";

contract Destroyable is Ownable {

    function destroyContract () public onlyOwner {
        selfdestruct(msg.sender);

}

HelloWorld

pragma solidity 0.7.5;

import "./Ownable.sol";

import "./Destroyable.sol";


contract Bank is Ownable {

I realised that I was overcomplicating this exercise after doing some research, then looked at how other people did it and realised I was on the right path, just overcomplicating the selfdestruct. I have now taken notes for future reference.

jon_m · July 17, 2021, 9:31pm

Nice solution @Jackthelad

Just a couple of observations …

(1) You don’t need to include the require() statement in the terminate() function …

… because this exact same require statement is already within the inherited onlyOwner modifier, which has also been correctly applied to terminate(). So you are currently applying this same condition twice to the same function.

(2) This may just be a result of you mixing up different versions you’ve tried …

Implementing 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. However …

you have named the file containing your Destroyable contract Terminate.sol , but you’re importing Destroyable.sol which doesn’t exist in the code you’ve posted;
if Bank only inherits Destroyable, you don’t need to import Ownable.sol as well.

You can use either, as long as it is passed to selfdestruct() as a payable address type. This is so that the contract’s remaining ether balance can be transferred to that address when the contract is destroyed. To receive ether an address must be payable. Some of the alternative ways to code this are:

(1) Use msg.sender (as you have done) to reference the contract owner’s address, because this is the only address that it can ever represent in your terminate() function, as a result of the onlyOwner modifier restricting who can call this function in the first place. Prior to Solidity v.0.8, msg.sender is payable by default, and so that’s why you haven’t needed to explicitly convert it in your solution (this course is based on v0.7.5).
However, 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));

(2) Use owner to reference the contract owner’s address. To be able to do this, you also need to do either of the following:

Declare the owner state variable in Ownable with a payable address type, instead of with the default, non-payable address type;
If you only need the contract owner’s 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 terminate() public onlyOwner {
          selfdestruct(payable(owner));
      }

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

jon_m · July 17, 2021, 10:01pm

Hi Matej,

We don’t need it there at all in this particular version of the Bank contract… which is the original point I was making when I said it should be removed. But that doesn’t mean you can exclude the Ownable contract from your inheritance structure, because, as you now realise, Bank needs to inherit Destroyable, and Destroyable needs to inherit Ownable so that the onlyOwner modifier can be applied to the destroy() function (so it is available in Bank, but can only be accessed by the contract owner).

We would only want to restrict the withdrawal of funds to the owner, whilst allowing any address to deposit funds into the contract, in scenarios such as: donating funds to a project; users depositing funds within a dapp where, once deposited, are restricted to being spent on transactions related to that dapp only; or where there are other mechanisms for withdrawal provided by functions other than withdraw(). The different permutations are endless.

But in this particular contract I think it is clear that individual account holders need to be able to call withdraw() to withdraw their ether, otherwise I think the bank would either have lots of complaints, or no customers

jon_m · July 18, 2021, 12:38am

An excellent assignment solution @gregD

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.

Also, well done for realising that you needed to make the owner address payable , so that selfdestruct() is called with a payable address and, therefore, any remaining ether in the contract can be transferred to the owner when selfdestruct() is triggered.

All of the user account balances (including the owner’s) — which were recording each user’s share of the total funds held in the contract, and stored in the mapping — will be 0 because the contract has been destroyed. The remaining ether in the contract is transferred to the owner’s external address, which is the one that deployed the contract initially (the address that was displayed in the Account field in the Deploy & Run Transactions panel on the left of the Remix user-interface). This is the address which was assigned to the owner state variable via the constructor and, therefore, the address which is passed to selfdestruct() and, therefore, the address which ultimately receives the remaining ether in the contract when it is destroyed.

Before calling closeContract(), check what the owner’s external address balance is, and make sure you also know how much ether should still be in the contract at this point. Then, after calling closeContract(), you should see the owner’s external address balance increase by that amount.

You can also add the following function to your Bank contract, which will allow you to view the total contract address balance at any point in time, instead of having to try to remember it.

function getContractBalance() public view returns (uint){
    return address(this).balance;
}

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

jon_m · July 18, 2021, 1:58am

Hi @Andre_Mark,

That’s a good way to tackle these challenges. Try to do as much as you can yourself first, even if it’s a struggle. Do your own research, if you need to. Then, finally, have a look at other students’ solutions, and the comments and feedback they’ve received, to either check your finished solution, or to get some further guidance if needed.

You are nearly there with your solution…

Your Ownable and Destroyable contracts are both well-coded — apart from a missing closing curly bracket for Ownable, which I’m sure is just a copy-and-paste slip And you have also coded their parent/child inheritance relationship correctly

However, there is an issue with how you’ve coded Bank as a derived contract within the overall inheritance structure. You’ve imported Ownable.sol and Destroyable.sol, but only added Ownable to the contract header. This means that Bank will only inherit Ownable, so when we deploy Bank, the selfdestruct() functionality will not be available to the contract owner. In order for this to happen, Bank needs to inherit Destroyable.

In fact, Bank only needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

At the moment you have a hierarchical inheritance structure with Bank and Destroyable as two separate derived contracts (with no inheritance relationship between them), which both inherit the same, single base contract (Ownable). But this doesn’t work for the reasons I’ve explained above.

Have a go at modifying the start of your Bank.sol file and the Bank contract header for this — that’s the only bit you need to change.

Just let me know if you have any questions

Jackthelad · July 18, 2021, 4:16am

Thanks again, I applied the changes after I had a think about them for a while.

I was wondering when using 0.7.5 would you encounter an error using

selfdestruct(payable(msg.sender));

or would it just be redundant?

Do such errors occur between contracts? If a base contract is compiled in an older version than the child contract or vice versa can issues emerge

I already got confused with a version issue in the multi-sig wallet not realizing there was a specific fallback function keyword in later versions not used in the earlier ones.

TLDR; Is it worth the hassle to try the course with the later versions of solidity, or just stick to 0.7.5 for now and educate myself later?

Cheers again.

ZED91 · July 18, 2021, 11:40am

“Ownable.sol”:

pragma solidity 0.7.5;

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”:

import "./Ownable.sol";

pragma solidity 0.7.5;

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

“HelloWorld.sol”:

import "./Destroyable.sol";

pragma solidity 0.7.5;

contract HelloWorld is Destroyable {

}

gregD · July 18, 2021, 12:02pm

That makes a lot more sense now, thanks for the help!

Andre_Mark · July 18, 2021, 5:30pm

OK, gotcha, I see, I just did it and it worked, actually doing this resolved something that I raised just now so I will close that comment now

suhan_kim · July 19, 2021, 3:11pm

//Ownable.sol
pragma solidity 0.7.5;

contract Ownable {
address payable owner;

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

constructor(){
    owner = msg.sender;
}

}

//Destoryable.sol
pragma solidity 0.7.5;

contract Ownable {
address payable owner;

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

constructor(){
    owner = msg.sender;
}

}
//Bank.sol
pragma solidity 0.7.5;
import “./Destoryable.sol”;

contract Bank is Destoryable{
[Code body];
}

GahuckImGoofy · July 20, 2021, 3:12pm

Ownable.sol:

pragma solidity 0.7.5;

contract Ownable {
    address public owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _; //run the funciton
    }
    
    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.sol:

pragma solidity 0.7.5;

import "./Destroyable.sol";

contract Bank is Destroyable {...

I definitely took some time on this and read the earlier answers by other people. I just want to be certain when I click on getBalance after I close the contract - I should get 0 back, correct?

DHash · July 20, 2021, 7:47pm

Owner contract:

pragma solidity 0.7.5;

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

Destroyable contract:

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

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

Bank contract

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

contract Bank is Destroyable{
  
    mapping(address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    event balanceTransfer(uint amount, address depostiedFrom, 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, "Insuficient balance");
        balance[msg.sender] -= amount;
        msg.sender.transfer(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 send money to yourself");
            
            uint previousSenderBalance = balance[msg.sender];
            
            _transfer(msg.sender, recipient, amount);
            
            emit balanceTransfer(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;
                
    }
    
}

jon_m · July 21, 2021, 3:16am

Hey Jack,

It would still compile without an error. It would just be redundant. This is because payable() itself isn’t new syntax introduced with v0.8, and so was also available in v0.7. The change with v0.8 is that we need to use it to convert msg.sender to a payable address where we need msg.sender to represent a payable address.

Where you have an inheritance structure, you are only compiling and deploying the derived contract, which includes the inherited functionality from the base contract. So, you would only be using one compiler version. The version pragma that we have to include at the top of each separate file is a way to declare which compiler version (or range of compiler versions) can be used to compile the code in that particular file. It is a way to ensure that the Solidity code contained within a particular file is only compiled based on a version which its syntax is compatible with. As breaking changes to the syntax aren’t introduced with every new release, a range of compiler versions can be declared in the version pragma by using symbols such as ^

The compiler will throw an error if the version pragma in either the file containing the derived contract or the file containing the base contract is incompatible with the compiler version. You can test this for yourself in Remix by experimenting with different version pragma declarations in the files containing your Bank, Destroyable and Ownable contracts. You then need to have the file containing the Bank contract displayed as the active tab in order to see the results of compiling the Bank contract as the derived contract. In Remix, the compiler version should automatically switch to the one declared in the version pragma of the file displayed as the active tab, but you can easily change this manually, if necessary, using the dropdown at the top of the Solidity Compiler panel.

Here’s a link to the relevant section in the Solidity documentation:
https://docs.soliditylang.org/en/latest/layout-of-source-files.html#pragmas

I think the only change between v0.7 and v0.8 that affects the course and assignment code is msg.sender changing from a payable address by default to a non-payable address by default. I don’t think it would take too long to make copies of your files, change the version pragmas and let the compiler point you to where you need to wrap msg.sender in payable(). It’s always beneficial to revisit and rework previous code, but this is also something you can easily just introduce from now on.

As a reference, here is a link to the section in the Solidity documentation which lists all of the breaking changes introduced with v0.8
https://docs.soliditylang.org/en/latest/080-breaking-changes.html

adykor · July 21, 2021, 4:16am

pragma solidity 0.7.5;

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

contract Bank is Ownable, Destroyable {...

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


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

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

jaejang22 · July 21, 2021, 4:34am

pragma solidity 0.7.5;

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

pragma solidity 0.7.5;

import "./ownable.sol";

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

pragma solidity 0.7.5;

import "./destroyable.sol";

contract Bank is Destroyable {
    
    
    mapping(address => uint) 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(balance[msg.sender] >= amount);
        balance[msg.sender] -= amount;
        msg.sender.transfer(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, "Balance not sufficient");
        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;
    }
    
}