Inheritance Assignment

Nice solution @Rafael_Albuquerque :ok_hand:

You have done a good job of using the appropriate syntax for Solidity v.0.8.1 :+1:

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 is paid/transferred to the owner when selfdestruct() is triggered.

Do you mean in the Destroyable contract, as follows?

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

…meaning that you can leave owner as a non-payable address (in Ownable) as follows?

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

If you define an address state variable as payable, then it will be payable whenever it is referenced without having to explicitly convert it each time. This is why in your posted solution you are able to call selfdestruct() as follows:

selfdestruct(owner);

However, if your address state variable only needs to be payable in specific situations, it is better practice (and more secure) to store it in the contract state as a non-payable address, and then convert it locally whenever it needs to be payable e.g.

selfdestruct(payable(owner));

… or use a suitable alternative such as   payable(msg.sender)

Can we also 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).

Let me know if anything is unclear, if I have misunderstood your question, or if you have any further questions :slight_smile:

Awesome, thanks. Yeah it took a bit of error message deciphering to figure out I needed to make the owner address payable for this to work but got there in the end. Thanks again for the reply!

1 Like

function close() public onlyOwner { //onlyOwner is custom modifier
selfdestruct(msg.sender); // owner is the owners address
}

The code I created, I have it on my bank contract but you can create a new contract I guess, works the same

Hi @vili,

Yes, but the aim of this assignment is to practise inheritance. So now try moving (i) the selfdestruct() functionality to a separate contract (Destroyable) in a separate file, and (ii) the contract ownership functionality to a separate contract (Ownable) in another separate file (if you haven’t already done so). You should have 3 separate contracts/files in total. Then add the necessary code so that Bank inherits the selfdestruct() and Ownable functionalities, and both are available when we just deploy Bank i.e. you 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.

This is correct, but once you’ve set up your inheritance structure, can we see how you have implemented this close() function, and what code you’ve added for the inheritance, by posting your Ownable and Destroyable contracts, and the start of your Bank.sol file, and Bank contract header?

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

Ok, it was not clear that you wanted the derived contracts as well; the assignment only asked for the Destroyable contract. I’ll get back to you.
Regarding the internal visibility: the intention was to prevent anyone from directly calling
Destroyable.close() if it is overloaded in a derived contract. Now, I did some research and it seems like solidity, unlike other languages, does not allow this anyway. If a function is overloaded there is no way to explicitly call the parent contract implementation from the outside (like remix), correct?

Hi @decrypter :ok_hand:

Like other statically-typed programming languages, Solidity does allow overloading. Although, from your post, I’m not entirely sure we are using the term to refer to the same thing. Have a look at this discussion about overloading (polymorphism) in Solidity, and let me know if this is what you are referring to.

I’m not clear about what exactly you mean by…

It is the derived contract, not the parent contract, that we are deploying. When the derived contract is deployed, close() will be available via inheritance. It’s the onlyOwner modifier that is already preventing anyone from calling close() and destroying the derived contract. And for the contract owner to call close(), it needs to have public visibility (if it had external visibility, then it wouldn’t be inherited).
However, if you post your derived contract with your specific implementation, then I will be able to see from your code what it is you intend, and can comment more meaningfully.

Here, it seems like you are using “overloaded” to mean “inherited”. Overloading in Solidity has nothing to do with where a function can be called from — that’s determined by the visibility. To be clear, if we only deploy Bank, then close() will only be available to be called on the Bank contract. Even though Destroyable’s functionality is inherited, it is not our intention to also deploy the Destroyable contract. But even if we were to deploy both Bank and Destroyable, if close() was called on Destroyable, instead of on Bank, then Destroyable would be destroyed, not Bank. But it makes no sense to deploy Destroyable, because its purpose is to separate out certain core functionality, and to allow this same core functionality to be implemented in contracts that inherit it, and not for it to be used separately for its own sake.

Anyway, to avoid confusion and talking at cross purposes, I think the best thing is to see both contracts for your original solution :slight_smile:

Hello Jon,

I’m not quite sure you intended to send this to me as I’m not decrypter …?

Kind regards -
yestome / Andreas

1 Like

@jon_m thx a lot for your detailed comments and corrections. One can feel that you do that with love :wink:

1 Like
pragma solidity 0.7.5;

contract Ownable{
    
    address payable owner;
    
    constructor(){
        owner = msg.sender;
    }
    
    modifier ownerOnly{
        require(msg.sender == owner, "You aren't the owner");
        _;
    }
}

This is going to check the owner and deal with everything about being owner.

pragma solidity 0.7.5;

import "./Ownable.sol";

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

And this is for making the self destruct only possible by the owner.

pragma solidity 0.7.5;

import "./Destroyable.sol";


contract Bank is Destroyable{
    
    mapping(address => uint) balance;

    event depositSuccessful(uint amount, address indexed _from);
    event withdrawSuccessful(uint amount, address indexed _to);
    
    event fundsTransfered(uint amount, address indexed _from, address indexed _to);
    
    
    function deposit() public payable returns(uint){
        balance[msg.sender] += msg.value;
        emit depositSuccessful(msg.value,msg.sender);//sending out log that deposit is done
        return balance[msg.sender];
    }
    
    function withdraw(uint _amount)public returns(uint){
        require(balance[msg.sender] >= _amount, "Not enough funds");
        balance[msg.sender] -= _amount;
        msg.sender.transfer(_amount);
        emit withdrawSuccessful(_amount,msg.sender);
        return balance[msg.sender];
    }
    
    function getBalance() public view returns(uint){
        return balance[msg.sender];
    }
    
    function transfer(uint _amount,address _to) public{
        require(balance[msg.sender] >= _amount, "Not enough balance");
        require(msg.sender != _to, "Can't send funds to yourself?");
        _transfer(msg.sender,_to,_amount);
        emit fundsTransfered(_amount, msg.sender, _to);
    }
    
    function _transfer(address _from,address _to, uint _amount) private{
        balance[_from] -= _amount;
        balance[_to] += _amount;
    }
        
    
    
}

And this is the latest code after all is done. I hope it’s right :slight_smile:

1 Like

Assuming that a contract “Ownable” contains the logic defining a specific owner

pragma solidity 0.7.5;

import "./Ownable.sol"

contract Destroyable is Ownable{

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

Hey Andreas,

In my reply to decrypter, I linked to our Polymorphism discussion, as it was relevant. I think you may have got a notification as well, because it is your discussion thread (which you started with your question) that I linked to. I’ve checked my reply to decrypter and it’s definitely sent to him and not you, so I think you just got notified.

Thanks for the message though — strange things can happen sometimes :crazy_face:

Of course :heart_eyes: :blush:

Jon -
relevant yes it is - thanks for the notification !

1 Like

import “./Ownable.sol”;
pragma solidity 0.7.5;
contract Destroyable is Ownable
{
function destroy() public onlyOwner
{
address payable receiver = msg.sender;
selfdestruct(receiver);
}
}

1 Like

Hello;

hopefully this is correct:

contract destroyable is ownable {
    function close() public onlyOwner { 
  selfdestruct(owner);  
    }
}

Take care

An excellent assignment solution, and overall three very well and accurately coded contracts @Deini-Atakan :muscle:

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.

And 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 paid/transferred to the owner when selfdestruct() is triggered.

Your fundsTransfered event and corresponding emit statement are also well coded. The emit statement is in the correct position within the transfer function, and it logs relevant information when a call to the transfer function has executed successfully.

The only code that seems to be missing are the 2 lines for the assert statement in the transfer() function… but maybe you had this in earlier versions of your Bank contract and it just got lost somewhere along the way :relaxed:

You’re making great progress, Atakan!

1 Like

Hi @Kippie,

Your code doesn’t compile because of the missing semi colon at the end of your import statement…

You should have got a red compiler error for this, and the error message explains what the problem is. Apart from this, your Destroyable contract is well-coded, but it does rely on you having made an additional change in Ownable. When the selfdestruct() function is called it should automatically transfer the ether balance remaining in the contract to the address it is called with. For this to happen, the address (in your case, owner ) needs to be a payable address. As you haven’t converted owner to a payable address locally, within the boom() function body, we do also need to see your Ownable contract to confirm whether owner is payable or not when it is passed to selfdestruct().

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. This is a key part of your solution, because the aim of this assignment is for our Bank contract to inherit both the contract destruction and 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).

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

Thanks for these kind words, it really encourages me. I had the assert function in my code but as you said, I removed it for some reason that I don’t remember right now, should I add it back? I understood the importance of that function but is it more like a safety precaution or is it only just to be sure?

1 Like

So I first created an ownable contract to specify that only the owner of the contract can call the call the destroy() function.

pragma solidity 0.7.5;

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

Then created the destroyable contract which inherits from the ownable contract in order to take owner as a parameter in the selfdestruct() function

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

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

And lastly created a sample contract to destroy that inherits from the destroyable contact and therefore also inherits from the ownable contract.

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

contract myContract is Destroyable{
    
    function hello(string memory input) public pure returns(string memory){
        return input;
    }
    
}

My main doubt on this assignment was the payable functionality added to the owner address. I understand that the selfDestruct() function should take a payable address as an argument because the error messages on Remix alerted me about this, but otherwise no idea.

Also I was reading on one of the posts from @Guy and found it quite interesting (if I understood correctly)that you could also simply pass msg.sender as an argument to the selfdestruct() function, an skip the ownable contract all together (at least for the destroyable contract).

2 Likes

thanks jon i’m going to do it again and impliment those changes. btw sometimes i read the error message and understand other times i’m confused on what it is suggesting

1 Like