Inheritance Assignment

jon_m · March 6, 2022, 5:25pm

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

jon_m · March 6, 2022, 6:35pm

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

jon_m · March 6, 2022, 6:15pm

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!

Xabier22 · March 9, 2022, 7:53pm

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;  
    }

  
}

0xtravis · March 11, 2022, 1:53am

Ah, I didn’t know that. Thanks for the tip Jon!

jon_m · March 13, 2022, 12:36am

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.

DeMarco_Saunders · March 15, 2022, 4:08pm

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.

DeMarco_Saunders · March 15, 2022, 4:22pm

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 {}

oioii1999 · March 16, 2022, 9:28am

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 {
    // ...
}

jon_m · March 17, 2022, 3:22am

Hi @DeMarco_Saunders,

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

jon_m · March 17, 2022, 3:34am

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!

oioii1999 · March 17, 2022, 12:44pm

Thanks for your explanation! @jon_m

Previously I thought the default keyword for class variable is private. Does it mean we have to explicitly write the keyword private, when we intend to hide the base class variables that cannot be accessed by the derived class? Is it the same usage when we write class functions with inheritance?

Thank you and look forward to your reply

Daniel_O · March 18, 2022, 2:05pm

@filip @jon_m There is something which is not clear to me so far. Maybe I missed smth while I was doing the ETH 101 course.

Coming from a Java background the course was pretty straightforward and the inheritance assignment the same, however, there’s a few things which I can’t seem to understand.

In the inheritance assignment it is specified that “You should be able to deploy the Bank contract, perform some transactions and, finally, destroy the contract, transferring any remaining ether held in the contract to the contract owner.”

To transfer the remaining ether held in the contract, this is being achieved by calling the close() function. I can clearly see the address being updated with the amount once close is being called.

contract Destroyable is Ownable {

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

}

What I don’t understand is how the close() method (once it reaches the selfdestruct) knows the amount of ether held by the contract.

The only reference of the amount/balance of Ether deposited in the smart contract is in the
mapping (address => uint) balance; which declared in the Bank.sol file.

So again, how does the selfdestruct knows about it - it’s not being passed a parameter or something similar to the close() method. I guess is some Solidity magic being done behind the scene, but again as a Java guy, it doesn’t make any sense to me so I need it to ask this…I hope someone will be able to shed some light here…

Cheers.

karendj · March 20, 2022, 11:14am

Contract: ownable.sol

pragma solidity 0.8.7;

contract Ownable {
    address owner;

    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }

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

Contract: destroyable.sol

pragma solidity 0.8.7;

import "./ownable.sol";

contract Destroyable is Ownable {

    event DestroyContract (string _message);

    function destroy() onlyOwner public payable {
        emit DestroyContract ("Self destruct initiated.");
        address payable ownerAddress = payable(owner);
        selfdestruct(ownerAddress);
    }

}

Contract: bank.sol

pragma solidity 0.8.7;

import "./destroyable.sol";

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

    event depositDone(uint256 _amount, address _depositedTo);
    event amountWithdrawn(uint256 _amount, address _withdrawnFrom);

    function deposit() public payable returns (uint256) {
        balance[msg.sender] += msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }

    function withdraw(uint256 amount) public returns (uint256) {
        require(
            balance[msg.sender] >= amount,
            "You cannot withdraw more than what you have!"
        );

        balance[msg.sender] -= amount;
        payable(msg.sender).transfer(amount);

        emit amountWithdrawn(amount, msg.sender);
        return balance[msg.sender];
    }

    function getBalance() public view returns (uint256) {
        return balance[msg.sender];
    }

    function transfer(address recipient, uint256 amount) public {
        require(msg.sender != recipient, "You cannot transfer to yourself!");
        require(balance[msg.sender] >= amount, "Balance insufficient.");

        uint256 previousSenderBalance = balance[msg.sender];

        _transfer(msg.sender, recipient, amount);

        assert(balance[msg.sender] == previousSenderBalance - amount);
    }

    function _transfer(
        address from,
        address to,
        uint256 amount
    ) private {
        balance[from] = balance[from] - amount;
        balance[to] = balance[to] + amount;
    }
}

jon_m · March 21, 2022, 4:40am

Hi @oioii1999,

Sorry for not replying sooner!

No, the default visibility for state variables is internal

Yes, if by “hiding”, you mean that you want the state variable in the base contract/class to only be accessible from within that same base contract i.e. it won’t be inherited.
If you do not include any visibility keyword in a definition of a state variable in the base contract/class, then it will have internal visibility by default, meaning that it will be accessible both from within that same base contract and from within the derived contract/class i.e. it will be inherited. But unlike a state variable defined with the public visibility keyword, it won’t be accessible from an external service (e.g. a web3 client) or from an external smart contract.

Yes, except that with functions the visibility keyword must always be included in the function header. Functions have no default visibility, and if you omit the visibility keyword, you’ll get a compiler error.

The other difference with functions is that as well as public, private and internal visibility, we can also define them with external visibility. An external function in a base contract/class will still be inherited by the derived contract/class, but when the derived contract is deployed, the function can only be called from an external service (e.g. a web3 client) or from an external smart contract i.e. it cannot be called from within the deployed contract itself. An external function cannot be called from within the same contract/class or from within a derived contract/class.

I hope that’s answered your questions and made things clearer. But do let me know if you have any further questions

jon_m · March 21, 2022, 6:48am

Hi @Daniel_O,

Sorry for the delay in replying!

The balance mapping records the individual user balances. These balances perform an internal accounting role, and record each user’s share of the contract’s total Ether balance i.e. each user’s entitlement to withdraw Ether from the contract.

The Ether is held in the contract address balance. When a user calls the deposit() function to deposit Ether in the contract, the fact that the function has been marked payable enables the Ether value sent to the function (input into the Value field in the Deploy & Run Transactions panel in Remix) to be added automatically to the contract address balance. It might help to think of payable in a function header 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.

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.

A user can retrieve their individual share of, or entitlement to, the total contract balance by calling getBalance(). However, if we want to retrieve the actual amount of Ether held in the contract at a particular point in time (the contract address balance) then we need to add a function which returns this instead e.g.

function getContractBalance() public view returns(uint) {
    return address(this).balance;
}

The withdraw() function does the opposite of what the deposit() function does: it deducts Ether from the contract address balance and sends it to the caller’s external address (msg.sender). It does this using Solidity’s in-built transfer method, which is called on a payable address …

<address payable>.transfer(uint256 amount);

Just as the execution of a function marked with the payable keyword results in the EVM performing low level operations which add any Ether value sent to the function to the contract address balance …
… when Solidity’s in-built transfer method is executed, it results in the EVM performing low level operations which (i) deduct an amount of Ether from the contract address balance which is equivalent to the uint256 argument the transfer method is called with, and (ii) transfer and add this amount of Ether to the external address the transfer method is called on.

In the withdraw() function, the line …

balance[msg.sender] -= amount;

… ensures that the same value deducted from the contract address balance, and transferred out of the smart contract, is deducted from the individual user’s balance in the mapping, in order to keep track of their share of the total funds held in the contract, which is necessary to be able to ensure that they can’t withdraw more Ether than they are entitled to.

Just as the payable keyword and the transfer method are each pre-defined within the Solidity lanaguage and result in the low level operations described above …
… when Solidity’s in-built selfdestruct method is executed, it results in the EVM performing low level operations which (i) transfer an amount of Ether equivalent to address(this).balance to the external address argument which the selfdestruct method is called with (in our case the contract owner’s address), and then immediately (ii) destroy the smart contract address(this) by removing it from the Ethereum blockchain.

I hope that clarifies things. But just let me know if anything is still unclear, or if you have any further questions

Daniel_O · March 21, 2022, 7:56am

@jon_m Thanks very much for the detailed answer. I was actually thinking that there is some low level EVM magic behind the scene, that’s why I asked the question. I mean, it made sense what was happening and I kinda guessed here and there (given my java background and familiarity with the JVM) but just wanted to understand exactly how it works on the Solidity side. Now everything make sense, so thanks again for that (Really appreciate it). I am just about to start ETH 201 course today, really looking forward to that.

oioii1999 · March 21, 2022, 3:45pm

Really appreciate your patient explanation! @jon_m

I have learned a lot about the inheritance of class variables and functions.

jon_m · March 22, 2022, 1:11am

Hi @Daniel_O,

Glad the explanation was helpful … I know exactly what you mean when you can draw on your general experience to make an intuitive guess about what’s probably going on behind the scenes, but that it’s also good to get some confirmation and a more detailed explanation just to tie up any loose ends, put your mind at rest, and also just in case you’ve missed something important

Enjoy 201

jon_m · March 22, 2022, 3:13am

Hi @karendj,

This is a very well-presented solution which works and meets all of the main 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.

A few observations and comments about the code in your Destroyable contract …

(1) 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). So, your destroy() function shouldn’t be marked payable because it doesn’t receive Ether. Instead, it deducts Ether from the contract address balance and sends it to an external address (the contract owner’s). This is also why the withdraw() function isn’t marked payable.

The selfdestruct method within the destroy() function does require a payable address argument. This is so it can transfer the remaining Ether in the contract to this receiving address when the contract is destroyed. You have already ensured that this address argument is payable by converting the non-payable owner address using payable() in the line above…

In fact, a more concise alternative to using this additional ownerAddress variable is to call selfdestruct with owner directly, and convert owner to a payable address within the function call itself…

selfdestruct(payable(owner));

Have a look at this post for further details about the use of the additional local variable receiver in the model solution.

(2) Your DestroyContract event is certainly a good idea. Even though an event’s emit statement should be placed after the code associated with the event itself — in order to minimise the risk of the event being emitted erroneously — you are right that we can’t do this here. Once selfdestruct has executed, the contract will have been destroyed and its code removed from the blockchain; so, if the emit statement is placed after the call to selfdestruct, it can’t be reached and the intended event data will never be logged. The only solution is to do what you’ve done and place the emit statement before the call to selfdestruct.

It’s true that if selfdestruct failed, then the destroy() function would revert, including the emitted event, but we still need to carefully weigh up the benefits of emitting this event against the potential risk of emitting it before the actual event itself (contract destruction) has occurred.

The reason why I say that emitting an event is a good idea (at least in theory) is because of a serious drawback of using selfdestruct, the potential repercussions of which need to be fully understood before choosing to implement it. As you may or may not already have realised, after selfdestruct has been executed, successful calls can still be made to the destroyed contract’s functions! As a result of this, if a smart contract deployed on the mainnet is ever destroyed, all users need to be notified and informed that they must not send any more funds to that contract address. If a user doesn’t know that the contract has been destroyed, and goes ahead and sends funds to it (e.g. by calling the deposit function), then those funds will be lost. You can see this for yourself by calling deposit() with an Ether value. Remix will notify you that the transaction has been successful, and you will see that the Ether amount has been deducted from the caller’s external address balance. However, as the contract no longer exists, the Ether has been lost.

It’s worth bearing in mind that emitting an event doesn’t necessarily mean that users will see that infomation, and so may not actually prevent them from subsequently calling the destroyed contract’s functions, incurring a gas cost, and losing any Ether they might send. There would still need to be some form of communication to all users informing them of the contract’s destruction and what the consequences and implications of this are in terms of what they must and mustn’t do from now on.

Obviously, this is hardly an ideal situation! In practice, other types of security mechanisms, such as pausable or proxy contracts, are implemented to deal with these kinds of emergency situations, instead of selfdestruct. But this is out of the scope of this introductory course. You will learn about more advanced security techniques and practices if you go on to take the Ethereum Smart Contract Security Course.

Let me know if you have any questions