Ownable.sol
contract Ownable {
address owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
}```
Destroyable.sol
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner {
selfdestruct(msg.sender);
}
}
Bank.sol
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract Bank is Destroyable {
pragma solidity 0.7.5;
contract Ownable{
address public owner;
constructor(){
owner=msg.sender;
}
modifier onlyOwner{
require(msg.sender== owner);
_;
}
}
contract Destroyable is Ownable{
function destroyContract() public payable{
require ( owner == msg.sender, "You are not the owner");
selfdestruct(msg.sender);
}
}
contract Bank is Destroyable{
mapping(address => uint)balance;
// This function will deposit money to the contract
function deposit() public payable returns(uint){
balance[msg.sender]+=msg.value;
return balance[msg.sender];
}
//this function will withdraw the amount from the contract to the address calling this function
function withdraw(uint amount) public {
require(amount <= balance[msg.sender],"Balance is not sufficient");
msg.sender.transfer(amount);
balance[msg.sender]-=amount;
}
//this function will get the balance of address executing this function
function getyourBalance() public view returns(uint){
return balance[msg.sender];
}
//this function will get you the total balance of ether in your contract
function CheckBalanceofContract() public view returns(uint){
return address(this).balance;
}
}
Bank contract
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 returns (uint) {
balance[msg.sender] = 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);
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, "You do not have enough ETH");
require(msg.sender != recipient, "Error");
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;
}
}
Ownable
pragma solidity 0.7.5;
contract Ownable {
address owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
}
Destroyable
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner {
selfdestruct(msg.sender);
}
}
Another excellent solution, @LELUK911 
Your implementation of a multi-level inheritance structure is the best approach. Bank_Eth only needs to explicitly inherit Self_destroyed, because it will implicitly inherit Owner via Self_destroyed.
You have converted msg.sender to a payable address where the Solidity v0.8 syntax you are using requires this âŚ
The setOwner() function youâve added to the Owner contract is well coded and appropriate. You have correctly commented that the owner variable cannot be marked immutable if the owner address can be changed with the setOwner() function after deployment. Otherwise, you could definitely define owner as immutable, as your alternative code suggests, although it would also still need to be marked payableâŚ
address payable public immutable owner;
⌠unless you convert owner to a payable address only when it needs to be, within the destroyed() function itself e.g.
function destroyed() public OnlyOwner {
selfdestruct(payable(owner));
}
Let me know if you have any questions ⌠and keep up the great coding! 
An excellent solution @Techman120
Your implementation of a multi-level inheritance structure is the best approach. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.
Nice solution @JaimeAS 
Your implementation of a multi-level inheritance structure is the best approach. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.
Donât forget to specify the required compiler version for each separate file, by including a pragma statement in each one. This may just be a copy-and-paste error when posting your code, but if it isnât, the compiler should have generated orange warnings for omitting these.
It is not good practice to omit the pragma statement from any Solidity file, because we should always specify the Solidity compiler version (or range of versions) which our code is written to be compatible with.
Because the Bank contract inherits functionality from Destroyable and Ownable, even if the pragma statements are omitted from either or even both of their files, Remix will still attempt to compile Bank as a single contract together with the code it inherits. Bank will compile and deploy successfully as long as the code in both of the inherited contracts is compatible with the Solidity version declared in the pragma statement at the top of the file containing Bank. However, the compiler still generates orange warnings, highlighting that itâs best practice to include pragma statements in each file.
In the absence of a pragma statement, it seems that Remix will always try to find a suitable compiler version, although this doesnât seem to be 100% reliable.
So, in conslusion, always include a pragma statement at the top of every .sol file.
Let me know if you have any questions
Hi @imran82,
Your solution works and meets all of the assignment objectives Your implementation of a multi-level inheritance structure is also the best approach: Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.
A few observations:
(1)âYour destroyContract() function should not be marked payable, because it doesnât need to receive Ether from an external address to add to the contract address balance.
It might help to think of payable as the code that enables msg.value to be added to the contract address balance, without us having to add any further code for this. In the deposit function, this happens automatically because we have marked the function as payable . The line balance[msg.sender] += msg.value; then adds the same value to the individual userâs balance in the mapping, in order to keep track of their share of the total funds held in the contract.
In contrast, the destroyContract() function triggers the selfdestruct method which, in addition to destroying the contract, also deducts the total amount of Ether from the contract balance and sends it to the payable address argument (msg.sender) which selfdestruct is called with, and which in our case can only ever be the contract ownerâs address.
(2)âYou donât need to include the require() statement in your destroyContract() function âŚ
⌠because the inherited onlyOwner modifier is already available within Destroyable to add to the destroyContract() function header, and thereby prevent any address other than the contract owner from executing this function, destroying the contract, and receiving any Ether still remaining in the contract.
You could, however, add the error messageâ"You are not the owner"âto the require() statement in your onlyOwner modifier.
// Suggested improvements to address the points made in (1) and (2) above
function destroyContract() public onlyOwner {
selfdestruct(msg.sender);
}
(3) By including all three contracts within the same file, your solution works with only one pragma statement, and without having to add any import statements. However, best practice is to place each contract in a separate file â no matter how small any of them are â each with its own pragma statement, and to import the parent contract file(s) where this is necessary for the successful implementation of the inheritance structure. Doing this makes your code more modular and, therefore, also more re-usable and easier to develop further. Improved modularity and code re-usability are both key reasons for choosing to implement inheritance.
Let me know if anything is unclear, or if you have any questions
Hi @Arch.xyz,
Your code meets the assignment objectives. Your inheritance structure is well coded, but Bank only actually needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
However, your withdraw() function is no longer adjusting the userâs individual balance in the mapping, because it no longer includes â balance[msg.sender] -= amount;
This is obviously a serious bug, and needs to be corrected.
Youâve also 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!
Let me know if you have any questions
Destroyable.sol
pragma solidity 0.8.7;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner{
selfdestruct(payable(msg.sender));
}
}
bank contract.sol
pragma solidity 0.8.7;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable {
Hi @Wyse_Sousa,
Your code meets the assignment objectives. Your inheritance structure is well coded, but Bank only actually needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.
Because youâre using Solidity v0.8, you have correctly converted msg.sender to a payable address when passing it as an argument to selfdestruct âŚ
As you may already be aware, from Solidity v.0.8 msg.sender is no longer payable by default, which means it has to be explicity converted to a payable address when necessary e.g. when passed to selfdestruct as its payable address argument.
The transfer method also needs to be called on a payable address. So, if you havenât done so already, you also need to change the following line of code in your withdraw function in Bank âŚ
// from
msg.sender.transfer(amount);
// to Solidity v0.8 syntax
payable(msg.sender).transfer(amount);
Just let me know if you have any questions.
Destroyable.sol
pragma solidity 0.7.5;
import "./ownable.sol";
contract Destroyable is Ownable {
function close() public onlyOwner {
selfdestruct(owner);
}
}
Ownable.sol
pragma solidity 0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor() {
owner = msg.sender;
}
}
Bank.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable
My bank contract does not appear in the contract section above deploy, so I am not able to deploy the bank contract.
Ownable.sol
pragma solidity 0.7.5;
contract Ownable{
address public owner;
modifier onlyOwner{
require(msg.sender == owner);
_;
}
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(header)
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract bank is Destroyable, Ownable{
So I played around with this for a few hours, I got it to the way I liked it, but I kept getting an error at the self destruct line. So I went online trying to see if something was off with my syntax. I ended up adding some lines from the blog to see if that changed things, but then I started getting new errors on the constructor and the address payable private line. Can you see what my problem is in here? My guess is itâs redundant, but when I tried taking certain lines out I still got error messages. Here is the code below:
Ownable Contract Code:
pragma solidity 0.7.5; //SPDX-License-Identifier: SPDX-License-Identifier; unlicensed//
contract Ownable {
address owner;
modifier onlyOwner{
require (msg.sender == owner);
_; //run the function
}
}
Destroyable Contract Code:
pragma solidity 0.7.5; //SPDX-License-Identifier: SPDX-License-Identifier; unlicensed//
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 payable {
selfdestruct(owner);
}
}
Bank Contract Code:
pragma solidity 0.7.5; //SPDX-License-Identifier: SPDX-License-Identifier; unlicensed//
import âDestroyable.solâ;
contract Bank is Destroyable{
Thanks so much for all the awesome feedback and insights @jon_m!! Reading through all of this now and updating my contracts 
The level of detail is truly really helpful for me, I really appreciate it.
Hey @jon_m â thanks again for all the help above! Sorry for the delay, been heads deep in work the past month but now getting my Academy studies back on track
Made updates to my code, would you be able to check these over to let me know if Iâm on the right track on this? I totally saw what you mean know about how the onlyOwner modifier was on the withdraw functionâŚI was wondering why I could only withdraw as the contract deployer but thatâs why Iâve updated everything below, so looking forward to hearing what you think.
// SPDX-License-Identifier: None
pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable {
mapping(address => uint) balance;
// define event
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
function deposit() public payable returns (uint) {
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 returns (uint) {
require(balance[msg.sender] >= amount); // prevents withdraw of others' money
msg.sender.transfer(amount); // run the transfer function
balance[msg.sender] -= amount; // remove amount from balance before transfer occurs
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);
// governmentInstance.addTransaction(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;
}
}
// SPDX-License-Identifier: None
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable {
function destroy() public onlyOwner {
address payable receiver = msg.sender;
selfdestruct(receiver);
}
}
// SPDX-License-Identifier: None
pragma solidity 0.7.5;
// way to inhert from contract ownable for owner functionality
contract Ownable {
address internal 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;
}
}
Hi @MichaelAudire,
Thatâs because your Destroyable contract isnât compiling, and so your Bank contract isnât compiling either. If Bank isnât compiling you canât deploy it, so thatâs why itâs not appearing in the Contract field.
You are probably getting a compiler error that says either contract ownable.sol, or contract Destroyable.sol , has not been found. This is because your file names in the import statements are inconsistent in terms of whether they start with an upper- or lower-case letter. You need to make sure that they are exactly the same as your actual file names âŚ
but
and
Once you have corrected this, Bank still wonât compile because of another error in Destroyable, in your close() function âŚ
If you read the compiler error for this line, you will see that your owner argument in the selfdestruct() function call needs to be explicitly converted to a payable address. This is because the owner state variable in your Ownable contract is defined as a non-payable address; but the method selfdestruct requires a payable address argument to be able to transfer the remaining Ether in the contract to this address when the contract is destroyed.
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.
If you post your corrected code, Iâll take another look at it for you. But just let me know if you have any questions, or if you need any more help
Hi @Abel,
The code within the body of each contract (Ownable and Destroyable) is correct, but your inheritance structure is incorrectly coded in Bank. This means that your Bank contract wonât compile and so canât be deployed.
What the compiler error is saying for this line of code is that the order of the inherited contracts is incorrect based on the parent-child inheritance relationship youâve already defined between Ownable (parent) and Destroyable (child). So, if you change the order to âŚ
contract Bank is Ownable, Destroyable { ... }
⌠then Bank will compile. However, including both contracts in the Bank contract header is unnecessary. Bank only actually needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structureâŚ
contract Ownable { ... }
contract Destroyable is Ownable { ... }
contract Bank is Destroyable { ... }
Once youâve corrected this, youâll be able to deploy Bank and perform the different transactions mentioned in the assignment instructions, finishing with the contract owner destroying the Bank contract and transferring the remaining contract balance to their own external address.
Just let me know if you have any questions
Hi @Liquidbaker89,
Your implementation of a multi-level inheritance structure is well coded and is the best approach: Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable
Before you added all the additional code to Destroyable, this was probably because your owner state variable in your Ownable contract is defined as a non-payable address âŚ
The method selfdestruct in your close() function requires a payable address argument. This is so it can transfer the remaining Ether in the contract to this address when the contract is destroyed. But you were calling selfdestruct with owner , which referenced a non-payable address. The compiler error for this line would have told you that your owner argument in the selfdestruct() function needed to be explicitly converted to a payable address.
Marking the close() function payable doesnât make the owner argument payable. Only functions that receive Ether from an external address (in order to add it to the contract address balance) should be marked payable (e.g. the deposit function in Bank). The close() function doesnât receive Ether, it deducts Ether from the contract address balance and sends it to an external address.
Instead, you can make the argument passed to selfdestruct a payable address in one of three ways:
(1) If we only need the contract ownerâs address to be payable for the purposes of executing selfdestruct , we can leave the owner state variable in Ownable as a non-payable address type, and explicitly convert the address to payable locally, within the function where we actually need to use it as payable. We can even do this directly within the selfdestruct() function call itself, as follows:
selfdestruct(payable(owner));
(2) Or you can define the owner address state variable as a payable address type by adding the payable keyword âŚ
address payable owner;
Youâve done this by adding a duplicate owner state variable to Destroyable. Once youâve added a semi colon to the end of âŚ
⌠in order to correct the compiler error youâre getting on the following line (where the constructor is), youâll then get a compiler error for âŚ
This line throws an error, because Destroyable already inherits an owner state variable from Ownable. The error message tells you that owner has already been declared. So, you need to remove the owner state variable declaration from Destroyable and either make the one in Ownable payable, or explicity convert owner locally instead (as described in option 1 above). But if you define the owner state variable in Ownable as payable (option 2) you canât also define it with private visibility, because then it wonât be inherited. The owner state variable should be declared within Ownable and not Destroyable, because it is a fundamental part of the Contract Ownership functionality and we want to aim for code which is modular and therefore re-usable. The constructor also needs to be within Ownable, otherwise the contract ownerâs address will not be assigned to the owner state variable on deployment, and the onlyOwner modifier will also be useless.
(3) For security, we want to ensure that only the contract owner can call close() and trigger selfdestruct. Currently, your close() function has no such restriction. By applying the onlyOwner modifier, only the owner address will be able to call this function. This will also mean that msg.sender in this function will only be able to reference the contract ownerâs address, giving us a third option of passing msg.sender to selfdestruct as its argument instead of ownerâŚ
selfdestruct(msg.sender);
If you post your corrected code, Iâll take another look at it for you. But just let me know if anything is still unclear, if you have any further questions, or if you need any more help
Hi everyone,
a quick comment on the destroyContract() function, please feel free to share your thoughts.
I thought that after changing the ownerâs balance, the function should have the owner withdraw() all the contract funds before calling destroy() and therefore destroying the contract, otherwise the funds will just be stuck in the contract and nobody would be able to access them.
Here is my solution:
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
constructor() {
owner = msg.sender;
}
modifier onlyOwner {
require(msg.sender == owner, " Operation allowed only to contract owner");
_;
}
}
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.7.5;
import "./ownable.sol";
contract Destroyable is Ownable {
function destroy() internal onlyOwner {
selfdestruct(owner);
}
}
And just the beginning of the Bank contract. Everything else is the same.
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.7.5;
import "./destroyable.sol";
contract Bank is Destroyable {
mapping(address => uint) balance;
event withdrawal(address indexed client, uint amountWithdrawn, uint newBalance);
function destroyContract() public payable onlyOwner {
balance[owner] += (address(this).balance - balance[owner]);
withdraw(balance[owner]);
destroy();
}
function withdraw(uint amount) public returns (uint) {
require(balance[msg.sender] >= amount, "Insufficient funds to withdraw");
msg.sender.transfer(amount);
balance[msg.sender] -= amount;
emit withdrawal(msg.sender, amount, balance[msg.sender]);
return balance[msg.sender];
}
}
Hey @InterstellarX,
Youâre very welcome! Iâm glad you found the feedback really helpful
The modifications youâve made mean that your Bank contract will now work as it should, and it meets all of the assignment objectives Your inheritance structure is correctly coded, but now consider this comment âŚ
i.e.
contract Ownable { ... }
import "./Ownable.sol";
contract Destroyable is Ownable { ... }
import "./Destroyable.sol";
contract Bank is Destroyable { ... }
Youâve correctly removed your duplicate close() function from Bank, but you didnât need modify your original close() function in Destroyable âŚ
Calling selfdestruct() with msg.sender directly, is a concise alternative to using the additional receiver variable which youâve added to this function in your latest version âŚ
Have a look at this post for further details about the use of this additional local variable (receiver) in the model solution for this assignment.
withdraw() function in Bank
Anyone with funds in the contract can now withdraw their Ether
But youâve changed the order of the statements in the function body, which now makes the contract vulnerable to re-entrancy attacksâŚ
Your comments are correct, but the second one now doesnât make sense because youâve changed the orderâŚ
Remember this?
Here is the original order of your statements in the withdraw() function body, which was correct in terms of reducing security risk âŚ
You should also consider improving your balanceWithdrawn event âŚ
And finally, regarding your emit statement for the transfer event ⌠Generally speaking, itâs probably better to place an emit statement after an assert statement if there is one.
I hope you find these final comments useful as well 