Inheritance Assignment

Hi Donnie,

This is now correct, so yes, you must have misunderstood…

Here, I was telling you that, earlier, you had correctly imported Destroyable.sol into helloWorld.sol, but then you removed it.

The changes you’ve made to Destroyable are also correct :ok_hand:

Where are you seeing this error message? This will always be displayed in orange under Deployed Contracts in the Deploy & Run Transactions panel before you have clicked on the orange Deploy button. Before clicking on this button you need to make sure that:

  • all 3 contracts compile without any errors;
  • helloWorld.sol is open on your screen;
  • you have selected HelloWorld from the dropdown menu in the Contract field (just above the Deploy button).

I have copies of the exact same contracts you have posted, and have made the same changes I’ve told you to make and checked that you’ve made them… and my HelloWorld deploys no problem.

Are you importing the exact same file names in terms of exact spelling, capital/lower case letters etc.? e.g. if the Destroyable file is destroy.sol then you need to import "./destroy.sol" and not "./Destroyable.sol"

Another problem could be that you haven’t got all 3 files within the same folder. The file paths…

"./Destroyable.sol"
// or
"Destroyable.sol"

… will only locate the file if it is in the same folder as the file you are importing it into. You can check this in Remix in the workspace you’re using in the File Explorers tab (at the top of the menu). If you have created any workspace sub-folders, and you haven’t got all 3 .sol files in the same folder, then you need to ensure that they are. I haven’t found a way to move files in Remix between folders, so you would need to create a new file in the correct folder, then cut and paste your code into the new file. Make sure you delete the old file, and that your new file has the same name as the old one.

If none of the above solve the problem, then post all of your code in all 3 files, and tell us exactly what names you have saved each file as.

Hi @decrypter,

This is looking promising, but 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).

In addition, for the owner to destroy Bank, the owner address needs to be able to call close() to trigger selfdestruct(). This requires a call from an external service such as Remix. By giving close() internal visibility, you are right that it will be inherited by the derived contract, but without seeing your derived contract (which I assume is Bank) there is no way for us to know how you have ensured that close() can be called by the owner. In actual fact, by using internal you overcomplicate the situation, and create a need for additional code to be added to the derived contract. Instead, by making close() public, it would still be inherited by the derived contract and also directly available to external services, without the need to add any supporting code to facilitate this.

Well done for making sure your owner address is 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.

Just to make the code more interesting in terms of the inheritance, how would your solution look if you separated your contract ownership functionality and contract destruction functionality into 2 separate files — giving you 3 files in total?

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

pragma solidity 0.8.4;

import "./ownable.sol";

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

Destruct.sol

contract Destruct {

address payable public owner;

function DestroyContract() internal {
    selfdestruct(owner);
}

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

Child file


import "./Destruct.sol";

contract Bank is Destruct {

mapping (address => uint) balances;

function deposit() payable public {
    balances[msg.sender] += msg.value;
}

function getbalance() public returns(uint) {
    return(balances[msg.sender]);
}
function destroy() public {
    require(msg.sender == owner);
    DestroyContract();
}

}

1 Like

Screen Shot 2021-05-09 at 7.40.59 PM

here’s my tree.

from here, I click compile
Screen Shot 2021-05-09 at 7.43.25 PM

When I click on the deploy button, the tag says that I have no contract instances to interact with.
Screen Shot 2021-05-09 at 7.48.48 PM

Hi @Alex_Carr,

You’ve got the right idea, but the code you have posted won’t compile because it’s incomplete:

(1) This modifier is incomplete.

(2) You are missing a constructor to assign the owner address to the state variable owner on deployment.

(3) Your contract has no closing bracket.

(4) contract must be written with a lower case c. Have a look at this post which explains how to format your code before posting it in the forum. As well as looking better, and making your code more readable, formatting it like this will help to avoid errors such as this one.

Your destroy() function is good, and will do what it’s supposed to once you’ve sorted out the issues with the onlyOnly modifier. But 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).

Just to make the code more interesting in terms of the inheritance, how would your solution look if you separated your contract ownership functionality and contract destruction functionality into 2 separate files — giving you 3 files in total?

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

Nice solution @benlive :ok_hand:

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

Hi @BenB,

This is a good effort :ok_hand: The contracts compile, and when Bank is deployed it inherits the ownership and destruction functionality from Destruct. Also, well done for making sure your owner address is 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.

However, because the destroyContract() function is available in Bank via inheritance, there is no need to write an additional function (destroy) in Bank to be able to call destroyContract() and trigger selfdestruct() once Bank has been deployed. By changing the visibility of destroyContract() from internal to public , if you remove destroy(), you will see that destroyContract() now becomes available to call from Remix. You have probably already realised that this modification also requires the require statement to be moved to destroyContract() in order to ensure that only the owner address is able to destroy the contract. The fact that you don’t need this additional function in the derived contract, highlights an important advantage of inheritance: it helps to avoid code duplication, while maintaining modularity.

Just a couple of other observations:

You only need the parentheses if you are returning multiple values…

return balances[msg.sender];
// This is all that's needed when returning one value

Your compiler should have given you an orange warning, indicating that you should make this a view function, because it is only reading the contract state, but not modifiying it.

To make the code more interesting in terms of the inheritance, how would your solution look if you separated your contract ownership functionality and contract destruction functionality into 2 separate files — giving you 3 files in total?

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

1 Like

Hey Donnie,

Your file names look correct, and are all in the same main workspace folder.

You shouldn’t need to click Compile again, as you have Auto compile turned on, which should be compiling as you code. From what I can see you don’t have any compiler errors, just a warning (but it should still deploy OK with that). Is helloWorld.sol open on your screen? That shouldn’t really be a problem though, BUT you have Destroyable selected in the Contract field in the Deploy & Run Panel. Change that to HelloWorld. You need to click on the arrow and select the contract you want to deploy from the dropdown.

It’s strange, though, because it should still deploy Destroyable instead of HelloWorld when you click on Deploy — which isn’t what you want, but it shouldn’t still say Currently you have no contract instances to interact with. I don’t mean to state the obvious, but are you sure you’ve clicked properly on the orange Deploy button? Are you having this same problem with any of your other contracts?

jon_m for president!

Thanks
Ben

2 Likes

:rofl: :sweat_smile: Thanks for the vote of confidence, Ben!

1 Like
pragma solidity 0.7.5;

import "./Ownable.sol";

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

it wasn’t in the same folder. Problem solved, I am ready to move on. I have a goal. I want to build something. But first, I need to finish the new class.

celebrating!!

1 Like

TOTALLY INELEGANT, however compiles. I attempted more concise coding that did not work. How frequently do I need to explicitly mark addresses as payable, everytime they are sent/received?!

pragma solidity 0.7.5;

//create a new contract called Destroyable, which will allow any contract inheriting from it 
//to call a function to self destruct that should only be available for the contract owner.

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

contract Destroyable is Ownable{
    
    function closeContract(address payable owner) public payable onlyOwner(msg.sender){
        
        selfdestruct(owner);
    }
}

contract Bank is Destroyable{
    
    function endAll(address payable user) public payable {
    closeContract(user);
    }
}
contract destroyable_1{
    address public owner;
    
    modifier onlyOwner{
        require(msg.sender == owner);
        _;
    }
    
    constructor(){
        owner = msg.sender;
    }
    
    function closeSC() public payable onlyOwner {
        selfdestruct(msg.sender); 
    }
}
pragma solidity 0.7.5;

import "./Destroyable_1.sol";

contract transferFunction is destroyable_1{
    
    mapping(address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    event balanceTransferred(address from, address to, uint amount);
    
    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 send money to yourself");
        
        uint previousSenderBalance = balance[msg.sender];
        _transfer(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
        emit balanceTransferred(msg.sender, recipient, amount);
    }
    
    function _transfer(address from, address to, uint amount) private {
        balance[from] -= amount;
        balance[to] += amount;
    }
}
1 Like

Congratulations! :champagne: :clinking_glasses: :muscle:

Hi @jon_m
Thanks for the feedback! I actually missed to add the bank.sol file, which was naturally required as part of the solution, so it was a huge mistake.
From the first point, I actually had the Destroyable contract in the same file that I had the Ownable one (the file was called “destroyable.sol”):

pragma solidity 0.7.5;

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

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

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

If I would have each file in separate files (which in fact is the best practice), I must have started “Destroyable.sol” in the following way

pragma solidity 0.7.5;

import "./Ownable.sol";

I had the following content on my bank.sol file, where the most important changes related to the assignment are the importing of destroyable.sol and making the Bank contract a derived contract from Destroyable.

bank.sol

pragma solidity 0.7.5;

import "./Destroyable.sol";
contract Bank is Destroyable
{
    mapping(address => uint) balance;
    event balanceAdded(uint amount, address indexed receiver);
    event transferCompleted(uint amount, address indexed sender, address indexed receiver);
    event depositAdded(uint amount, address indexed sender);
    event withdrawCompleted(uint amount, address indexed receiver);
    
    function deposit() public payable returns (uint)
    {
        uint previousBalance = balance[msg.sender];
        balance[msg.sender] += msg.value;
        assert(balance[msg.sender] == previousBalance + msg.value);
        emit depositAdded(msg.value, msg.sender);
        return balance[msg.sender];
    }
    function withdraw(uint amount) public returns (uint)
    {
        require(balance[msg.sender] >= amount, "Insufficient funds");
        uint previousBalance = balance[msg.sender];
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        assert(balance[msg.sender] == previousBalance - amount);
        emit withdrawCompleted(amount, msg.sender);
        return balance[msg.sender];
    }
}
1 Like

Is there something I did not understood or any smart contract developer can exit with the money of everyone thx to that selfdestruct function?

1 Like

pragma solidity 0.7.5;

import “./Ownable”;

contract Destroyable is ownable{

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

}

// that was my answer, and I checked later your solution. I am not sure to understand perfectly you declare a “receiver” payable address variable. Didn’t you mention that to withdraw an address doesn’t not need to be “payable”. also isn’t everything already included in the “selfdestruct” predefined function?
Thx a lot for your lights

1 Like

Nice solution @AustinBurks :ok_hand:

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