Hi Juan,
This relates to a future lesson about external contracts. It’s crept into the solution code for this assignment by mistake, and so you can ignore it for the purposes of this assignment. When you are eventually introduced to the Government contract, the meaning of this line of code should become clear… or maybe you’ve already got to that lesson now… 
A very well presented solution, Dominykas 
Your implementation of a multi-level inheritance structure is also the best approach, because it is more streamlined: Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.
Yes… you shouldn’t use an address literal, here.
The address argument which is passed to selfdestruct() determines which address the contract’s remaining ether balance is sent to when it is destroyed; and so, in terms of security, one logical choice for this is the contract owner’s address. However, the contract owner’s address is set on deployment by the constructor, and so we need to ensure that, whichever this address is, that it’s the only address that can be passed to selfdestruct() when it is triggered.
msg.sender ensures this, because access to your destroyContract() function is restricted by the onlyOwner modifier (inherited from Ownable) which only allows the contract owner’s address to call this function, trigger revert and initiate a transfer of the remaining funds to their own external wallet address for safekeeping.
An alternative is to call selfdestruct with owner instead of msg.sender. Both will always reference the same address, but owner is defined as a non-payable address in Ownable:
To receive ether, an address needs to be a payable address. So you would either need to define owner as a payable state variable in Ownable:
address payable public owner;
… or explicity convert it locally whenever you need to use it as a payable address e.g.
selfdestruct(payable(owner));
Prior to v.0.8, msg.sender is payable by default, and so that’s why you don’t have to explicitly convert it here:
selfdestruct(msg.sender);
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));
I hope that makes sense. Just let me know if you have any further questions.
You’re making great progress 
pragma solidity 0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
pragma solidity 0.7.5;
import “./Ownable”;
contract Destroyable is Ownable {
function destroyContract() public onlyOwner {
selfdestruct(owner);
}
}
Ohh yes I see, I’m actually on that lesson now haha. Thanks!
Good solution @walter_w
Can we also see what modifications you’ve made to the start of your derived contract file, and contract header, for the inheritance?
Hi @Maia,
It’s difficult for me to review your solution, because there are several things that don’t make any sense…
(1) It’s not clear whether both of your contracts are in the same file or separate files.
(2) You’ve imported the Ownable.sol file into itself!
Ownable is the parent contract and it doesn’t inherit another contract. We only need to import a file when the file it contains is being inherited.
(3) Your Bank contract inherits Destroyable, but where is your code for the Destroyable contract? If it’s in a separate file to Bank, then it also needs to be imported.
(4) Your Bank contract references the onlyOwner modifier and the owner variable, but both of these are in the Ownable contract, and according to your code as it currently stands (excluding Destroyable), Bank does not inherit Ownable.
Some additional observations…
a) The idea of the assignment is to place the selfdestruct() functionality within a separate contract called Destroyable, and for this to be inherited by Bank so that it is available when Bank is deployed, but without having to include the actual function code within Bank.
b) The function that contains selfdestruct() cannot also have the name selfdestruct.
c) 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. You are passing owner , which is defined as a non-payable address in your Ownable contract, and so needs to be converted to a payable address. There are a couple of alternative ways to do this.
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. In this assignment the aim is to 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).
If you want to benefit from this course, I would strongly recommend that you go back over the lessons, and take your time to understand, practise, think through and experiment with the code in each section before moving on to the next. You’ve moved through the course much too quickly and you haven’t given yourself time to properly understand the assignments or to reflect on your assignment feedback.
Here is a list of some learning and study activities, which you may find useful when working on assignments, and I think they could help you get more out of our programming courses…
Anyway, I hope this advice has been helpful. Just let me know if you have any questions
ok, I have made many mistakes on this session in general. I have to go over the material again, get it right and repost everything according to the guidelines. Thank you so much for your time and guidance here, I truly appreciate every minute you spend here in order for me truly understand this subject and get it right. I’ll make the requested corrections.
Thank you so much,
Maia
Good solution @thecryptoargie
Can we also see what modifications you’ve made to the start of your derived contract file, and contract header, for the inheritance?
Hi @yestome,
You’ve nearly got it, but there are a couple of errors which the compiler should be highlighting…
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, but yours is currently non-payable. The error message you get in the compiler tells you what the problem is. If you don’t know, or can’t find out how to convert owner to a payable address (there are alternatives) then have a look at other students’ solutions, and the feedback and suggestions they’ve received, posted in this discussion topic.
The imported file will only be recognised if it includes .sol
Can you correct your code for these issues?
As Bank is the derived contract, and therefore the contract we are deploying, 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.
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. 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.
Let me know if anything is unclear or if you have any questions
pragma solidity 0.8.1;
import "./Ownable.sol";
contract Destroyable is Ownable {
function close() public onlyOwner { //onlyOwner is custom modifier
selfdestruct(owner); // `owner` is the owners address
}
}
Now the contract Bank need a modify only in inheritance:
contract Bank is Destroyable { ... }
Hi @Adrian_Zumba,
Your Destroyable contract is looking good, but can we also see your Ownable contract? We need to see Ownable as part of your solution, because passing owner to selfdestruct() relies on you having made a couple of additional changes in Ownable. If you haven’t made any modifications to Ownable, then you are passing a non-payable address to selfdestruct(). 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.
The code you’ve added to your Bank contract header for the inheritance is correct, and your implementation of a multi-level inheritance structure is also the best approach, because it is more streamlined: Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. But don’t forget you also need to import the file Destroyable.sol (assuming they are both in separate files).
Let me know if anything is unclear, or if you have any questions
I think I’ve got it put together now…
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable{
function killContract() public onlyOwner{
selfdestruct(owner);
}
}
and here’s the Ownable…
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor() {
owner = msg.sender;
}
}
and the Bank contract
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract Bank is Destroyable {
mapping(address => uint) balance;
event depositDone(uint amount, address indexed depositedTo);
event transferred(uint amount, address depositedFrom, 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, "Insufficient Balance Available");
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, "Insufficient Balance Available"); //require
require(msg.sender != recipient, "You may not transfer to the same address.");//require
uint priorSenderBalance = balance[msg.sender];
_transfer(msg.sender, recipient, amount);
assert(balance[msg.sender] == priorSenderBalance - amount);//assert
emit transferred(amount, msg.sender, recipient);
}
function _transfer(address from, address to, uint amount) private {
balance[from] -= amount;
balance[to] += amount;
}
}
An excellent assignment solution, and overall three very well and accurately coded contracts @Randallcrum
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 is paid/transferred to the owner when selfdestruct() is triggered.
You’re making great progress!
This was a little confusing mostly because I wasnt sure how to tell if the self destruct is working but I think i got it. 1. Do we only need to include “pragma solidity 0.7.5;” in one of the contracts or should i include it in each one, right now i only have in in my “bank.sol” file.
2. Destroyable inherits from Ownable, so in my bank.sol file i removed import ownable and put import destroyable .
pragma solidity 0.7.5;
import "./Destroy.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 onlyOwner 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;
}
}
`contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_; // run the function
}
constructor(){
owner = msg.sender;
}
}`
import "./Ownable.sol";
contract Destroyable is Ownable {
function close() public onlyOwner {
address payable reciever = msg.sender;
selfdestruct(reciever);
}
}
Nice solution @JonBal
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.
You should declare the Solidity compiler version in each file, before the contract(s). You should have got an orange compiler warning for each file where it is missing. The warning messages themselves specify what should be included.
Note that the compiler version needs to be declared for each file. So if you have all of your contracts in the same file, you only need one pragma solidity ... declaration. Although it is better practice to place each contract in a separate file.
Calling selfdestruct() should result in the following 2 outcomes:
Removal of the contract code and its data from the EVM. You can check that this has happened by first performing some transactions before destroying the smart contract. Call a getter to prove to yourself that data is stored in your smart contract as a result of those transactions. Then call selfdestruct(). You will notice that one of the disadvantages of selfdestruct() is that successful calls can still be made to the destroyed contract’s functions. This is a serious problem if ether is sent to the contract, because it will be lost. However, it also means that after selfdestruct() has been called, you can call a getter, and if it returns a zero value (where previously it had returned a non-zero value for a piece of data stored in the smart contract when it was still deployed) then this is proof that the contract has been destroyed.
If the contract address has a positive ether balance when selfdestruct() is called, this remaining balance will be automatically transferred to the address argument selfdestruct() has been called with. That’s why this argument needs to be a payable address. So, if you ensure that more ether has been deposited in your Bank contract (by any addresses) than has been withdrawn, you should see the contract owner’s address balance increase by the remaining contract balance when it calls selfdestruct(). This is much easier to check if you add a getter which returns the current contract balance (which will always be equivalent to the sum of the individual account holder balances at any one time) e.g.
function getContractBalance() public view returns(uint) {
return address(this).balance;
}
If you call getContractBalance() immediately before selfdestruct() is called, the returned contract balance is the same amount you should then see the owner’s address balance increase by, when selfdestruct() is called, if selfdestruct() executes as it should.
Just one additional observation…
This is correct, but the additional step of assigning msg.sender to a local variable (receiver) is not actually necessary. Assigning msg.sender to a payable address variable does not convert it to a payable address, because in Solidity v.0.7 it is already payable by default (although from v.0.8 it is non-payable by default). So, you can pass msg.sender to selfdestruct() directly, as follows:
selfdestruct(msg.sender);
Let me know if anything is unclear, or if you have any further questions
You’re making great progress and asking all the right questions!
Excellent! I totally see myself going back and forth over these assignments and it seems like the youtube channel is going to help a lot!
Thank you very much,
Maia
Do you mean this?
contract Bank is Ownable {
mapping (address => uint) balances;
event depositDone(address indexed depositedTo, uint ammount);
event transactionDone(address indexed fromSender, address indexed toRecipient, uint amount);
function deposit() public payable returns (uint) {
balances[msg.sender]+= msg.value;
emit depositDone(msg.sender, msg.value);
return balances[msg.sender];
}
Hi @thecryptoargie,
Yes, but the problem is that your Bank contract inherits the contract ownership functionality from Ownable, but it doesn’t inherit the selfdestruct() functionality. The aim of the assignment is for both to be 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).
Also, don’t forget that to inherit a contract in a different file you also need to import the file.
How would you fix this?
Thanks Jon for your hints and comments - Here is the amended code - hope this is correct now:
Ownable contract:
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
}
Destroyable contract:
pragma solidity 0.7.5;
import “./ownable.sol”;
contract Destroyable is Ownable {
function destroyContract() public onlyOwner {
selfdestruct(owner);
}
}
Bank contract
pragma solidity 0.7.5;
import “./destroyable.sol”;
contract Bank is Destroyable {
mapping(address => uint) balance;
event depositDone(uint amount, address indexed depositedTo); // max 3 parameters per event can be indexed
event transferComplete(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 getBalance() public view returns (uint){
return balance[msg.sender];
}
function withdraw(uint amount) public onlyOwner returns (uint) {
require(balance[msg.sender] >= amount, "insufficient balance");
balance[msg.sender] -= amount;
msg.sender.transfer(amount);
return balance[msg.sender];
}
function transfer (address recipient, uint amount) public {
require(balance[msg.sender] >= amount, "insufficient balance");
require(msg.sender != recipient, "You can't transfer money to yourself");
uint previousSenderBalance = balance[msg.sender];
_transfer(msg.sender, recipient, amount);
assert(balance[msg.sender] == previousSenderBalance - amount);
emit transferComplete(msg.sender, recipient, amount);
}
function _transfer(address from, address to, uint amount) private {
balance[from] -= amount;
balance[to] += amount;
}
}
pragma solidity 0.7.5;
contract bank{
mapping(address => uint)balance;
address owner;
event depositDone(uint, address indexed depositedTo);
modifier onlyOwner{
require(msg.sender == owner);
_;
}
constructor(){
owner = msg.sender;
}
function close() onlyOwner{
selfDestruct(owner);
}
function deposit()public payable returns(uint){
balance[msg.sender] += msg.value;
emit depositDone(msg.value, 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 self”);
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;
}
}