Ownable file.
Destroyable file
Bank contract
Destroyable.sol:
pragma solidity 0.7.5;
import "Ownable.sol";
contract Destroyable is Ownable {
function close() internal onlyOwner {
selfdestruct(owner);
}
}
Ownable.sol:
pragma solidity 0.7.5;
contract Ownable {
address payable public owner;
modifier onlyOwner {
require(msg.sender == owner);
_;
}
constructor() {
owner = msg.sender;
}
}
Finally I did add a new function to my Bank contract. It just seemed right to have the destroyable contract have its’ function be internal 
Bank.sol:
ragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";
contract Bank is Ownable, Destroyable {
// lots of other stuff
function destroy() public {
close();
}
}
Looking back I didn’t make the destroy function in the Bank contract onlyOwner and that could most likely save some gas for anyone that tried? I just don’t feel like it was safe leaving Destroyable contract’s function of close without internal and onlyOwner modifier.
Just curious what would a normal pattern be?
1 Like
ownable.sol
contract Ownable {
address public owner;
constructor() {
owner = msg.sender;
}
modifier onlyOwner {
require(msg.sender == owner);
_; // run the function
}
}
destroyable.sol
import "./ownable.sol";
contract Destroyable is Ownable {
function destroy() public {
require(msg.sender == owner, "ACCESS DENIED.");
selfdestruct(payable(owner));
}
}
bank.sol
import "./destroyable.sol";
contract Bank is Destroyable {
1 Like
HelloWorld:
pragma solidity 0.7.5;
import "./Destructable.sol";
contract HelloWorld is Destructable {
Ownable:
pragma solidity 0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //runs the function
}
constructor(){
owner = msg.sender;
}
}
Destructable:
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destructable is Ownable {
function destroy() public onlyOwner {
selfdestruct(msg.sender);
}
}
I now see it’s a bit different from Filip’s solution, but deploys and works so I am happy 
1 Like
Bank
pragma solidity 0.7.5;
import “./Ownable.sol”;
import “./Destroyable.sol”;
Contract Bank is Ownable, Destroyable {
Ownable
contract ownable {
Address internal owner;
modifier onlyOwner {
require(msg.sender == owner);
_; //run the function
}
constructor (){
owner = msg.sender;
}
}
Destroyable
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
Hi @JonPag,
Apologies for the delay in giving you some feedback!
With your code as it is at the moment, your Bank contract won’t compile, and so can’t be deployed.
Your inheritance structure is almost there … you’re just missing an import statement in Destroyable.sol to import Ownable.sol
Your close() function in Destroyable is correctly coded, but the onlyOwner modifier in the function header will throw a compiler error because you haven’t defined this modifier anywhere. As the modifier’s code needs to compare the address calling a function with the contract owner’s address, you also need a constructor to ensure that the address which deploys the Bank contract is assigned to the owner state variable in Ownable. If your owner address state variable remained unassigned, as it would without the constructor, then even though your Bank contract would still compile, the onlyOwner modifier would always fail and cause the transaction to revert.
Having made the above modifications, you should then be able to compile and deploy your Bank contract and successfully perform all of the different assignment tasks.
It’s also worth mentioning that Bank actually only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. This will give you a multi-level inheritance structure.
Follow the instructions in the FAQ: How to post code in the forum, in order to properly format your code before posting it. This will make it clearer, easier to read. and easier to copy-and-paste if we need to deploy and test it. In addition, you should apply appropriate indentation to your code, because this will also improve its clarity, readability, and manageability.
And by the way, don’t forget to also post your solutions to these earlier assignments …
Events Assignment https://studygroup.moralis.io/t/events-assignment/28040?u=jon_m
This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course.
Transfer Assignment https://studygroup.moralis.io/t/transfer-assignment/27368?u=jon_m
This is the assignment from the Payable Functions section of the course, where you have the task of completing the withdraw() function.
Let me know if anything is unclear, if you have any questions, or if you need any further help with correcting your solution code 
Hi @ekr990011,
Your solution meets all of the assignment objectives 
Your inheritance structure is well coded, but you haven’t taken full advantage of it, because the same objectives can be met in a more concise way …
As you are aware, by giving your close() function in Destroyable internal visibility, it is inherited and available within Bank, but it cannot be called by an external address from Remix. This creates the need for an additional public function to be added to Bank, which can call close(), to enable the contract owner to trigger selfdestruct() by calling this “intermediary” function. However, an important reason for using inheritance is to reduce code duplication, and by giving the close() function public instead of internal visibility it will still be inherited, but it also removes the need for the additional destroy() function in Bank, because close() will now appear as a function-call button in the Remix panel and can therefore be called by the contract owner directly.
You could also give the destroy() function external visibility, which would still mean that it is inherited by Bank, and that it can be called externally from Remix, but not from within the Bank contract, which for the purposes of this assignment would be enough.
Additionally, by keeping all of the code and functionality related to contract destruction within a single, separate contract (Destroyable), you will improve the modularity and re-usability of your code (other important reasons for using inheritance) and keep your code more organised.
Adding the extra public function doesn’t have any security benefits, and if anything it is less secure due to the increased complexity.
This is a very good point, and would certainly be an issue with functions that are going to be called relatively frequently. This is why require() statements that validate inputs should be placed as early in the control flow as is practically possible. If require() fails, then the transaction will revert wherever it is placed; but any code executed before require(), and which will already have been executed before require() triggers revert, will result in less gas being refunded than if the check had been performed earlier in the control flow. This is because only the gas cost associated with the unexecuted operations after require() is saved; any gas already consumed executing code for the operations before and including require() is not refunded. You can probably see, therefore, that the earlier in the control flow require() is placed, the less the cost of gas will be if it throws and the transaction reverts.
However, as there is only a potential gas cost saving for transactions which revert due to invalid inputs, and as the individual saving per each of these failed transactions is most likely to be fairly insignificant, this will probably only be a relevant factor for functions that are expected to be frequently executed with invalid inputs which trigger revert. And this, of course, is definitely not the case with the destroy() and close() functions we are discussing here.
It’s also worth noting that Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. This will give you a multi-level inheritance structure.
Let me know if anything is unclear, or if you have any further questions
1 Like
Hi @RyanYeap,
Your solution meets all of the assignment objectives
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. However, you haven’t taken full advantage of the inheritance structure, because access to the destroy() function can be restricted to the contract owner in a more concise and efficient way …
Including your additional require() statement in the destroy() function in Destroyable does ensure that only the contract owner can access this function and trigger selfdestruct. However, an important reason for using inheritance is to reduce code duplication.
All of the modifiers in a base contract are also inherited by its derived contract(s), so onlyOwner in Ownable, which performs exactly the same check as the extra require() statement you’ve added to destroy(), is already available for us to apply to the destroy() function. Adding onlyOwner to the destroy() function header removes the need for the additional require() statement at the beginning of the function body, while still having exactly the same effect.
Additionally, by keeping all of the code and functionality related to contract ownership within a single, separate contract (Ownable), you will improve the modularity and re-usability of your code (other important reasons for using inheritance). It also keeps your code more organised, and makes it more readable and easier to manage…
function destroy() public onlyOwner {
selfdestruct(payable(owner));
}
If you want to keep the "ACCESS DENIED" error message (which is a good addition), then, provided you are happy for this to be the error message for all instances where onlyOwner applies this access restriction, you can add it to the require() statement within the modifier in Ownable …
modifier onlyOwner {
require(msg.sender == owner, "ACCESS DENIED.");
_;
}
Also, don’t forget to include a pragma statement at the beginning of each file — although I’m sure you will have included these in your own version of the code.
Let me know if anything is unclear, or if you have any questions.
//SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
contract Ownable {
address payable internal owner;
modifier onlyOwner {
require(msg.sender == owner);
_; // run the function
}
constructor() {
owner = msg.sender;
}
}
//SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
import "ownable.sol";
contract Destroyable is Ownable{
function destroy() public onlyOwner {
selfdestruct(owner);
}
}
//SPDX-License-Identifier: MIT
pragma solidity 0.7.5;
import "destroyable.sol";
contract Bank is Destroyable {
...
1 Like
Hey @DaanBoshoven,
This is a great solution 
It’s actually an improvement on the model solution because:
(1) In terms of the inheritance structure, your implementation of multi-level inheritance is the best approach. Bank only needs to explicitly inherit Destructable, because it will implicitly inherit Ownable via Destructable.
(2) Calling selfdestruct() with msg.sender directly — selfdestruct(msg.sender); — is a more concise alternative to using the additional receiver variable …
address payable receiver = msg.sender;
selfdestruct(receiver);
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.
Just let me know if you have any questions
1 Like
Hi @Joeyj007,
Did you manage to successfully compile and deploy your version of Bank? I’m asking, because the code you’ve actually posted has several syntax errors which throw compiler errors, but which may just be formatting errors when copy-and-pasting your code from Remix to here.
These are the errors …
(1)
But the derived contracts are trying to inherit a non-existent parent that starts with a capital letter, not lower case.
(2) Data types, such as address and contract , must start with a lower-case letter …
(3) Keywords, such as pragma and import , must also start with a lower-case letter …
(4) Quotes must be coded with the characters " " or ' ' and not “ ”
Apart from this your solution is correct.
A couple of other observations …
(i) It’s worth noting that Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. This will give you a multi-level inheritance structure.
(ii) Calling selfdestruct() with msg.sender directly — selfdestruct(msg.sender); — is a more concise alternative to using the additional receiver variable …
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.
Just let me know if anything is unclear, or if you have any questions about any of these points
An excellent solution @0xtravis
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.
Just one observation …
This code is correct. However, it’s worth pointing out that, unlike functions, the visibility of state variables defaults to internal when the visibility keyword is omitted; unlike public and private, the internal keyword is optional. So, you can code your owner state variable declaration more concisely …
address payable owner;
Both work, but including the internal keyword here is unnecessary. It was probably included in the model solution in order to highlight the fact that owner has internal visibility, and so will be available in the derived contract. In practice, we would probably only want to explicity include the internal keyword in a state variable definition for emphasis or clarity; for example, if omitting it risks anyone reading or working with the code thinking otherwise.
Just let me know if you have any questions.
Keep up the great coding!
1 Like
pragma solidity ^0.8.0;
contract Ownable{
address owner;
constructor(){
owner=msg.sender;
}
modifier onlyOwner{
require(msg.sender==owner);
_;
}
}
contract Destroyable is Ownable{
function destroyContract() public onlyOwner {
selfdestruct(payable(owner));
}
}
contract bank is Destroyable{
mapping (address=> uint256) balances;
mapping (address => mapping(address => uint256)) allowances;
function addBalance(uint _toAdd) public returns(uint256){
balances[msg.sender]+=_toAdd;
return balances[msg.sender];
}
function balanceOf(address _address) public view returns(uint256){
return balances[_address];
}
function allow(address _spender, uint _amount) public {
require (balances[msg.sender]>=_amount,"Insufficcient funds");
allowances[msg.sender][_spender]=_amount;
}
function transfer(address recipient, uint256 _amount) public{
require(balances[msg.sender]>=_amount,"Insufficient funds");
_transfer(msg.sender, recipient, _amount);// en vez de actualizar los balances creamos una funcion privada que lo haga y lo usamos siempre
}
function transferFrom(address _sender, address _recipient, uint _amount) external{
require(allowances[_sender][msg.sender]>=_amount, "You are not allowed to spend that amount");
_transfer(_sender, _recipient, _amount);
}
function _transfer(address _sender, address _recipient, uint _amount) private{
balances[_sender]-=_amount;
balances[_recipient]+=_amount;
}
}
1 Like
Ah, I didn’t know that. Thanks for the tip Jon! 
1 Like
Hi @Xabier22,
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
Your inheritance structure works if all 3 contracts are in the same file. However, better practice is to place each contract in its own separate file, so that Ownable and Destroyable can be re-used as independent modules for easy implementation of the contract-ownership and contract-destruction functionalities. That would also mean adding import statements.
Increasing code re-usability is an important reason for using inheritance.
You’ve made a great attempt at introducing some of your own functionality into your Bank contract, in order to enable users to allow other addresses to access a specific amount of their funds/tokens and make transfers up to this amount, or “allowance”, to addresses of their choosing (either to their own, or to other user addresses). Your implementation of a double mapping to manage these “allowances” is particularly impressive
Your code works, but when an address with an “allowance” calls the transferFrom() function:
- if the balance of the sender address is already lower than the “allowance” of the calling address, for example if the sender has already made some transfers after setting an “allowance” for the calling address; and
- if the calling address tries to transfer an amount lower or equal to their “allowance”, but greater than the balance of the sender address; then …
… we shouldn’t rely on the following line of code in the private function …
… to trigger revert, because of the resulting underflow in the sender’s balance in the mapping. An underflow would occur here because an unsigned integer cannot be a negative value, and so when the value reaches zero the subtraction continues from the highest integer available for the number of bits the uint value is defined with (in your case 256). Underflows and overflows (an overflow is when the opposite of an underflow occurs) only automatically trigger revert in Solidity from v0.8. To see the effect this would have caused using an earlier version of Solidity, change your pragma statement to …
pragma solidity 0.7.5;
… and see what the sender’s balance is after performing the transfer I’ve outlined above!
Instead, best practice is to ensure that such an input amount also causes a require statement to fail and revert the transaction at the earliest opportunity. For example, you could do this by adding an additional require statement at the beginning of your transferFrom() function …
require(balances[_sender] >= _amount, "Insufficient balance");
require(
allowances[_sender][msg.sender] >= _amount,
"You are not allowed to spend that amount"
);
… or by combining both require statements as follows …
require(
balances[_sender] >= _amount &&
allowances[_sender][msg.sender] >= _amount,
"Invalid amount"
);
It’s also important to highlight that, as well as the contract owner being able to destroy the contract, another assignment objective is to ensure that, when the contract is destroyed, all remaining funds in the contract are transferred out of the contract to the contract owner. Your solution enables the contract owner, and only the contract owner, to destroy the contract, but all funds/tokens held in the contract are then lost and cannot be retrieved.
The reason why selfdestruct takes a payable address argument is because, as well as destroying the contract, selfdestruct also automatically transfers the remaining contract Ether balance to the address argument it is called with (in our case, the contract owner’s address). This is all part of the pre-defined selfdestruct functionality. However, in order to demonstrate this, you need to have a Bank contract which allows deposits and transactions of Ether, rather than just some kind of token i.e. something like the one we’ve been developing during the second half of this course with a payable deposit() function, withdraw() function etc. You can still add your additional functionality to this kind of contract as well.
(See the footnote about the Events and Transfer assignments)
In fact, the addBalance() function that you currently have in your contract doesn’t actually make any practical sense, because any address can just create as many tokens as it wants out of thin air and add them to its individual address balance in the mapping. In reality, such tokens would be minted according to a set protocol, hard-coded into the smart contract. Having users deposit Ether in the contract (by way of a transfer from their external addresses to the smart contract address balance) means that our contract is managing Ether which already pre-exists within the Ethereum ecosystem (or at least that is what Remix’s JavaScript Virtual Machine is simulating).
The addBalance() function was only used earlier on in the course as a way to introduce and demonstrate some of the fundamental concepts and syntax of Solidity, and serves more of a convenient building block rather than something that would be used in practice. You will learn how to code and implement smart contracts in accordance with the accepted token standards in the 201 course which follows this one.
A couple of other comments about your Bank contract …
-
Users can retrieve individual balances, but they also need to be able to retrieve their “allowances”, otherwise they won’t know how much of another user’s balance they are entitled to transfer.
-
You are missing an additional require statement in the transfer() and transferFrom() functions to prevent transferring funds/tokens to the same address that is sending them. Such a transaction would waste gas for no reason.
Let me know if you have any questions about any of these points
Don’t forget to also post your solutions to these earlier assignments …
Events Assignment https://studygroup.moralis.io/t/events-assignment/28040?u=jon_m
This is the assignment from the Events video lecture, which is near the end of the Additional Solidity Concepts section of the course.
Transfer Assignment https://studygroup.moralis.io/t/transfer-assignment/27368?u=jon_m
This is the assignment from the Payable Functions section of the course, where you have the task of completing the withdraw() function.
Can anyone help me? I’m getting a strange error at the end of my bank contract and i have no idea why.
“from solidity:
newcontract.sol:42:1: ParserError: Expected pragma, import directive or contract/interface/library/struct/enum/constant/function definition.
}
^”
There’s an extra curly bracket at the end but when I remove it, it gives me errors on every single function in the contract so I do not know how to resolve this problem.
*Edit: I Just deleted everything and rewrote the code from a previous version and the error is gone.
1 Like
Destroyable.sol
pragma solidity ^0.7.5;
import './Ownable.sol';
contract Destroyable is Ownable {
function destroy() public onlyOwner {
selfdestruct (msg.sender);
}
}
Ownable.sol
pragma solidity ^0.7.5;
contract Ownable {
address public owner;
modifier onlyOwner {
require(msg.sender == owner );
_;
}
}
Bank.sol
pragma solidity 0.7.5;
import './Ownable.sol';
import './Destroyable.sol';
contract Bank is Ownable, Destroyable {}
Ownable.sol
pragma solidity 0.7.5;
contract Ownable
{
address payable internal owner;
constructor()
{
owner = msg.sender;
}
modifier onlyOwner
{
require(msg.sender == owner);
_;
}
}
Destroyable.sol
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable
{
function close() public onlyOwner
{
selfdestruct(owner);
}
}
Bank.sol
pragma solidity 0.7.5;
import "./Destroyable.sol";
contract Bank is Destroyable {
// ...
}
1 Like
To be honest, I’m not sure what the problem was with the error message you were getting, but I’m glad you managed to resolve it in the end 
Your Bank contract will compile and deploy successfully, and your inheritance structure is well-coded. However, without a constructor to assign the deployer’s address to the owner state variable, owner will forever hold a zero address (the default value for an unassigned address variable in Solidity). This means that the contract owner will not be able to destroy the contract or have any Ether remaining in the contract transferred to their external address, because the onlyOwner modifier will prevent any address that isn’t a zero address from calling the destroy() function.
Can you add a suitable constructor to the right contract?
When you have, you’ll be able to deploy Bank and perform all of the tasks outlined in the assignment instructions.
It’s also worth noting that Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. This will give you a multi-level inheritance structure.
Let me know if you have any questions, or if you need any further help to correct your code
An excellent solution @oioii1999
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.
Just one observation …
This code is correct. However, it’s worth pointing out that, unlike functions, the visibility of state variables defaults to internal when the visibility keyword is omitted; unlike public and private, the internal keyword is optional. So, you can code your owner state variable declaration more concisely …
address payable owner;
Both work, but including the internal keyword here is unnecessary. It was probably included in the model solution in order to highlight the fact that owner has internal visibility, and so will be available in the derived contract. In practice, we would probably only want to explicity include the internal keyword in a state variable definition for emphasis or clarity; for example, if omitting it risks anyone reading or working with the code thinking otherwise.
Just let me know if you have any questions.
Keep up the great coding!