Inheritance & External Contracts - Discussion

[jon_m]

Hi @Bitborn,

@Bitborn

Is it correct that i would have to add “payable” in the header of the withdraw function and transfer functions in order to withdraw and transfer money?

No… neither of these functions receive ether, so they don’t need to be marked payable. A function should only be marked payable when it needs to receive ether from an external address to add to the contract address balance e.g. the deposit() function. 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. 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.

However, we should not mark the withdraw() function as payable , because it doesn’t need to receive any ether from an external address and add it to the contract balance. If we make it payable, then, if the caller sends an ether value to the withdraw() function by mistake (as well as calling it with a withdrawal amount) then this ether would be added to the contract balance and potentially lost to the user: this is because the withdraw() function does not make an equivalent accounting adjustment to the mapping for such a deposit (only for withdrawals). However, the function is much securer if we leave it as non-payable, because then, if the caller makes the same mistake, the function will revert, and the ether won’t be sent.

Effectively, the withdraw() function does the opposite of what the deposit() function does: it deducts ether from the contract balance and sends it to an external address. It does this using the address member transfer (which is like a method called on a payable address)…

<address payable>.transfer(uint amount);

The separate transfer() function (not the special transfer method in the withdraw function) shouldn’t be marked payable either, for the same reasons I’ve already outlined for the withdraw() function. In fact, calling the transfer() function doesn’t involve any ether entering or leaving the Bank contract at all. Instead, it performs an internal transfer of funds (effectively, a reallocation of funds) between two individual users of the Bank contract. The net effect to the contract balance is zero, and the only change is to the individual user balances in the mapping, in order to adjust the amount each of the parties involved in the internal transfer is entitled to withdraw from the contract (their share of the total pooled funds) i.e. the recipient’s entitlement (balance) is increased by the same amount the sender’s entitlement is reduced.

---

@Bitborn

One other question as welll… I noticed the “withdraw” function in bank.sol does not adjust the balance after the withdrawal. Shouldn’t there be a line to do this so we can accurately reflect the balance, like balance[msg.sender] -= amount. Something like this?

Yes, you are right that we need this line of code. I’ve checked the solution code for the Inheritance Assignment in GitHub, and this line of code is missing in the withdraw() function in the Bank contract. This is an error. Well spotted!

msg.sender.transfer(amount)  transfers ether from the contract address balance to the caller’s external address, but it doesn’t adjust the individual user balances in the mapping. As I’ve explained above, these balances perform an internal accounting role, and record each user’s share of the contract’s total ether balance. So, just as we increase a user’s individual balance when they deposit ether in the contract, we need to do the opposite whenever they make a withdrawal.

---

If you still have difficulties depositing, withdrawing and internally transferring funds with your deployed Bank contract, you may find this post helpful. It lays out step-by-step instructions of how to properly check that all of your functions are working correctly, and it details what you should see happening and where, if it is.

You can check the Bank contract’s ether balance if you add the following function to your Bank contract (if you don’t already have it) …

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

Let me know if anything is still unclear, or if you have any further questions

[mcs.olive]

• (1) Your full code from the 2 files containing your Ownable and Destroyable contracts;

• (2) Only include your full code from the file containing your Bank contract if you have made modifications within the contract itself. Otherwise, just include the first few lines of code up to and including the contract header.

Although I inherited from Ownable as well as Destroyable, it would not be necessary as Destroyable alreads inherits from Ownable. I did it in order to have it explicit in this assignment.

// Ownable contract
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.7.5;

contract Ownable{

    address payable owner;

    modifier onlyOwner {
        require(msg.sender == owner);
        _; //run function
    }
    constructor(){
        owner = msg.sender;
    }
}

// Destroyable contract
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.7.5;

import "./ownable.sol";

contract Destroyable is Ownable {

    function selfDestroy() public {
        selfdestruct(owner);
    }
} 

// Bank contract 
// The first few lines of code up to and including the contract header.
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.7.5;import "./ownable.sol";

import "./destroyable.sol";

contract Bank is Ownable, Destroyable {

[jon_m]

Hi @mcs.olive,

Your solution is almost there… Everything compiles, your inheritance structure works, the contract can be destroyed by calling the selfDestroy() function in Destroyable, and on destruction any ether still remaining in the contract will be transferred to the contract owner’s external address You just have one problem to resolve…

At the moment anyone can call selfDestroy() and destroy the contract. The funds are safe, because only the contract owner can receive them. But the fact that any address can actually trigger selfdestruct is obviously not ideal

The owner address argument, which selfdestruct() is called with, determines which external address the remaining contract balance will be transferred to. It doesn’t place any restriction on which address can trigger selfdestruct() and destroy the contract.

What functionality, inherited from Ownable, can you implement in Destroyable, to ensure that only the contract owner can call selfDestroy() and trigger selfdestruct ?

@mcs.olive

Although I inherited from Ownable as well as Destroyable, it would not be necessary as Destroyable alreads inherits from Ownable.

Correct
Bank only needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

Just let me know if you have any questions.

[mcs.olive]

@jon_m
Thanks a lot for your comments.
I changed function selfDestroy in Destroyable contract so only the owner can execute it.

    function selfDestroy() public onlyOwner{
        selfdestruct(owner);
    }

[InterstellarX]

Hey Filip and Moralis Academy,

Great video on inheritance! Great to visually see what this means. Am I correct in understanding the following structure?

Base Contract (parent): HelloWorld.sol (with the bank contract)
Derived Contract (child): Ownable.sol (with the Ownable contract)

Also, am I correct in making a comparison between Base/Derived Contract as parent-child components in React? For example, you have a parent item that passes down props to the child within it for example? Is this a similar-enough comparison for example purposes?

[thecil]

@InterstellarX

Base Contract (parent): HelloWorld.sol (with the bank contract)
Derived Contract (child): Ownable.sol (with the Ownable contract)

Also, am I correct in making a comparison between Base/Derived Contract as parent-child components in React? For example, you have a parent item that passes down props to the child within it for example? Is this a similar-enough comparison for example purposes?

Yes and No, the Base contract will have access to Devired Contracts, but it will not pass any props, because its an inheritance, Base contract will be able to access all functionalities of Derived contract, but this does not mean that props or arguments can be sent to the Derived Contract.

Its more like a extra module for the Base contract than a parent-child components.

Hope it helps.

Carlos Z

[jon_m]

Hi @InterstellarX,

I can’t comment on any comparison with React, but I can confirm the following about Solidity …

Child/derived contracts are not “within” parent/base contracts. If anything, it’s the opposite (but only figuratively speaking)… When a derived contract is compiled, all of the functionality that it inherits from a parent contract is automatically included, and the result is a single set of bytecode, which is then deployed as a single smart contract at a single Ethereum address on the Ethereum blockchain.

The functionality that a derived contract inherits from its base contract(s) is the following:

• All functions, except for those with private visibility (although only public and internal functions will be accessible to call from within the derived contract itself).
• State variables with public or internal visibility.
• All modifiers.
• All events.
• Constructors

Inherited state variables will include any public or internal mappings or arrays. If these store struct instances, then the properties (or attributes, as they can also be called) of these struct instances will be available to be referenced using dot notation within the derived contract.

@InterstellarX

Base Contract (parent): HelloWorld.sol (with the bank contract)
Derived Contract (child): Ownable.sol (with the Ownable contract)

You’ve got this the wrong way round…
Ownable is the base/parent contract, which is inherited by the derived/child contract Bank. In fact, in the Inheritance Assignment we should ideally end up with the following multi-level inheritance structure:

// Ownable.sol
contract Ownable {...}  // Base/parent contract of Destroyable

// Destroyable.sol
import "./Ownable.sol"
contract Destroyable is Ownable {...}  /* Derived/child contract of Ownable
                                          AND base/parent contract of Bank */
// HelloWorld.sol
import "./Destroyable.sol"
contract Bank is Destroyable {...}  // Derived/child contract of Destroyable

And just to be clear in terms of what @thecil is explaining here …

@thecil

the Base contract will have access to Devired Contracts, but it will not pass any props, because its an inheritance, Base contract will be able to access all functionalities of Derived contract, but this does not mean that props or arguments can be sent to the Derived Contract.

With inheritance in Solidity, it’s the derived contract(s) that can access (and have available) certain functionality in the base contract. For example, the derived contract can reference a public or internal state variable, or call a public or internal function, in the base contract; but the base contract can’t reference any of the state variables, or call any of the functions, in the derived contract.

Let me know if anything is still unclear, or if you have any further questions

[JayM700]

1. What is the base contract?
   Also known as parent contract, it’s the contract that is inherited or derived into a base contract either through single inheritance or multiple inheritance.

2. Which functions are available for derived contracts?
   Any of the public, internal functions of the base contracts that are inherited.

3. What is hierarchical inheritance?
   It’s when a single base contract is derived in multiple contracts.

[fedev]

1. What is the base contract?
It’s a contract which is being inherited by children contracts, acting as a base for developing the subclasses.

2. Which functions are available for derived contracts?
Public and internal scoped functions (and state variables).

3. What is hierarchical inheritance?
It’s when a contract is used as a base contract by many children contracts, forming a kind of taxonomy.

[fedev]

Hi there is my solution for lessons/inheritance-assignment-2

./abc/Ownable.sol

pragma solidity 0.7.5;

contract Ownable {
  address public owner;
  constructor(){
    owner = msg.sender;
  }
  modifier onlyOwner {
    require(msg.sender == owner, "Owner only");
    _;
  }
}

./abc/DestroyableByOwner.sol

pragma solidity 0.7.5;

import "./Ownable.sol";

contract DestroyableByOwner is Ownable {
  function destroy() public onlyOwner {
    selfdestruct(payable(owner));
  }
}

./SimpleBank.sol

pragma solidity 0.7.5;

import "./abc/Ownable.sol";
import "./abc/DestroyableByOwner.sol";

contract SimpleBank is Ownable, DestroyableByOwner {
...
}

[Cosmin_Fechet]

1. What is the base contract?
   A base contract is the parent contract (the contract that is inherited).

2. Which functions are available for derived contracts?
   All public and internal scoped functions.

3. What is hierarchical inheritance?
   A single contract acts as a base contract for multiple derived contracts.

[jon_m]

Nice answers @JayM700

Just a couple of clarifications …

@JayM700

1. What is the base contract?
   Also known as parent contract , it’s the contract that is inherited …

Correct

@JayM700

… or derived into a base contract …

No … the base (or parent) contract is inherited by the child (or derived) contract. We can also say that the child contract is derived from the base (or parent) contract.

@JayM700

… through single inheritance or multiple inheritance.

… through any type of inheritance relationship: single, multi-level, hierarchical or multiple inheritance.

Just let me know if you have any questions.

[jon_m]

An excellent, and very well-presented solution @fedev

We can also streamline the inheritance structure by having Bank explicitly inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

Let me know if you have any questions about this point.

[Cosmin_Fechet]

Hello. Could someone help me, please? I have a issue when I try to get the information from “getTransaction” function in Government contract. It says “call to Government.getTransaction errored: VM error: invalid opcode.”

I am adding below the two contracts. Maybe is something I did wrong and I can’t seem to notice what.

Government.sol

pragma solidity 0.7.5;

contract Government{

    struct Transaction{
        address from;
        address to;
        uint amount;
        uint txId;
    }

    Transaction[] transactionlog;

    function addTransaction(address _from, address _to, uint _amount) external{
        transactionlog.push(Transaction (_from, _to, _amount, transactionlog.length));
    }

    function getTransaction(uint _index) public view returns (address, address, uint){
        return(transactionlog[_index].from, transactionlog[_index].to, transactionlog[_index].amount);
    }
}

Etherbank.sol

pragma solidity 0.7.5;

import "./Destroyable.sol";
import "./Ownable.sol";

interface GovernmentInterface{
    function addTransaction(address _from, address _to, uint _amount) external;
}

contract EtherBank is Ownable,Destroyable{

    GovernmentInterface GovernmentInstance = GovernmentInterface(0x9ecEA68DE55F316B702f27eE389D10C2EE0dde84);

    //storage - means permanent storage of data (persistant data)
    //memory - temporare data storage (is stored temporarely as long as the function executes)
    //calldata - similar to memory, but READ-ONLY
    
    mapping (address => uint)balance;
    
    event depositDone(uint amount, address depositedTo);
    event balanceTransfered(uint amount, address fromAddress, address toAddress);

    function deposit() public payable returns (uint){
        balance[msg.sender] += msg.value; //this line is not necessarly because the balance mapping has nothing to do with the value of the contract, it just keeps track of our internal balances (which address deposited which money)
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }

    function withdraw(uint amount) public returns (uint){
        require(balance[msg.sender] >= amount);
        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);
        return(balance[msg.sender]);
    }

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

    function transfer(address recipient, uint amount) public{
    require(balance[msg.sender] >= amount);
    require(msg.sender != recipient);
    uint previousSenderBalance = balance[msg.sender];
    _transfer(msg.sender, recipient, amount);
    
    GovernmentInstance.addTransaction(msg.sender, recipient, amount);

    assert(balance[msg.sender] == previousSenderBalance - amount);
    emit balanceTransfered(amount, msg.sender, recipient);

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

    }
}

[jon_m]

Hey Cosmin,

There’s nothing wrong with your code.

Whenever you redeploy Government, then it will have a different contract address, and so you also need to change the Government address assigned to your governmentInstance in this line of your code in Bank:

@Cosmin_Fechet

GovernmentInterface GovernmentInstance = GovernmentInterface(0x9ecEA68DE55F316B702f27eE389D10C2EE0dde84);

You need to change this address before you deploy Bank, so that it’s the same as the current address that your Government contract is deployed at. If you forget to replace the previous address of your external contract for the new one, then when you call Bank’s transfer() function either of the following can happen …

(1) The transaction completes successfully, but the addTransaction() function isn’t called on the Government contract that you currently have deployed and showing in the Deploy & Run Transactions panel. So, when you then call getTransaction() on your Government contract, you get the error message that you’ve been getting:

call to Government.getTransaction errored: VM error: invalid opcode.

This is because the transfer data from the transaction wasn’t passed to the addTransaction() function that you are expecting, and so wasn’t added to the transactionlog array that you are now trying to retrieve data from. You get this error message because there is no data at the array index you are trying to look up.

OR

(2) The transaction reverts, because the external function call fails, and you get the following default error message for revert, which doesn’t actually explain what the error is:

transact to Bank.transfer errored: VM error: revert.

The transaction has been reverted to the initial state.
Note: The called function should be payable if you send value
and the value you send should be less than your current balance.
Debug the transaction to get more information.

Forgetting to update the Government contract address is easily done, and catches most of us out the first time we make this mistake

I hope that resolves your issue, but let me know if you still experience any problems.

[Cosmin_Fechet]

@Cosmin_Fechet

GovernmentInterface GovernmentInstance = GovernmentInterface(0x9ecEA68DE55F316B702f27eE389D10C2EE0dde84);

Thank you very much for your help, Jon. I assumed I understood your remarks, but it seems that I didn’t and I don’t know why. I am sorry, but could you please help me again? I followed your steps and I still get the revert error. Please see below the steps I made.

1. I changed the address for the GovernmentInterface with a new one, but didn’t deploy the Etherbank contract yet:

GovernmentInterface GovernmentInstance = GovernmentInterface(0x617F2E2fD72FD9D5503197092aC168c91465E7f2);

2. I went into the Government contract and deployed it with the address 0x617F2E2fD72FD9D5503197092aC168c91465E7f2

3. Now I deployed Etherbank contract with the address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4

4. I deposited 2 ether with the address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, got the ballance 2000000000000000000.

5. Try to transfer 1000000000000000000 from the address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 to address 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2.

6. Got the error:

transact to Bank.transfer errored: VM error: revert.

The transaction has been reverted to the initial state.
Note: The called function should be payable if you send value
and the value you send should be less than your current balance.
Debug the transaction to get more information.

[jon_m]

Hey Cosmin,

You’ve got confused between external wallet addresses (user/deployer addresses) and contract addresses. The address that deploys the contract is not the same as the address the contract is then deployed at.

---

(1)

@Cosmin_Fechet

2. I went into the Government contract and deployed it with the address 0x617F2E2fD72FD9D5503197092aC168c91465E7f2

Do this first.
0x617…7f2 is the external address of the contract deployer.

(2) To get the contract address (the address it’s deployed at), go to the bottom of the Deploy & Run Transactions panel to the deployed contract, where it will say   GOVERNMENT AT   and then the address the Government contract is now deployed at. This is the address you need to copy and paste (use the icon to the right of the address) into your governmentInstance declaration in Bank …

@Cosmin_Fechet

1. I changed the address for the GovernmentInterface with a new one, but didn’t deploy the Etherbank contract yet:

GovernmentInterface GovernmentInstance =
GovernmentInterface(0x617F2E2fD72FD9D5503197092aC168c91465E7f2);

To create an instance of the Government interface, we need to declare where the contract (on which the interface is based) is deployed at. Otherwise, an external function call can’t be made to it. At the moment, your Bank contract is trying to call a function on the deployer’s external address which doesn’t exist.

---

(3) Continue with your steps 3 and 4

@Cosmin_Fechet

3. Now I deployed Etherbank contract with the address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4

4. I deposited 2 ether with the address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4, got the ballance 2000000000000000000.

(4) And your step 5 will now work, without throwing the error

@Cosmin_Fechet

5. Try to transfer 1000000000000000000 from the address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4 to address 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2.

(5) You can now call getTransaction() in Government with a transaction ID of 0, and retrieve the transaction data from the transaction log in Government.

---

Just let me know if you still have problems getting this to work.

[Cosmin_Fechet]

Thanks Jon. All good now. I appreciate your help

[Bitborn]

Thanks for the thorough explanation Jon. This was really helpful. There is just one other thing i am hoping you can answer regarding the transferFrom function… Supposing I approve someone to spend my funds. That person can then use the transferFrom to send my funds to whomever he wants correct?

So in the transferFrom header… the “from” address is the person i have approved to spend my funds? And the “to”/msg.sender" is where he is sending the funds i have given him the approval to spend? Or do i have something mixed up here?

[jon_m]

Hi @Bitborn,

I’ve tried to find a post of yours with your full Bank contract, but it doesn’t seem like you ever posted it… is that right? I wanted to see it because you mention a transferFrom() function header, and this doesn’t appear in the Bank contract used in the course videos/assignments.

Anyway, I’ll assume you mean the _transfer() helper function which is called from within the transfer() function…

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

This is a private function, and so can’t be called from outside the Bank contract. When the public transfer() function is called, the _transfer() function is then called from within the transfer() function in this line of code …

_transfer(msg.sender, recipient, amount);

As you can see, the msg.sender argument is passed to the _transfer() function as the address from parameter.

@Bitborn

So in the transferFrom header… the “from” address is the person i have approved to spend my funds?

So, no … it’s the caller of the public transfer() function…

function transfer(address recipient, uint amount) public { ... }

The caller of this function is the address which executes the internal transfer, which is effectively a reallocation of part, or all, of their own share of the total amount of ether held in the contract (recorded as their individual balance mapped to their address in the mapping) to another address of their choosing. No ether leaves the contract: the reallocated share is added to the other address’s individual balance (also mapped to their address in the mapping). This reallocation is performed in the _transfer() helper function.

@Bitborn

And the “to”/msg.sender" is where he is sending the funds i have given him the approval to spend?

The recipient argument is passed to the _transfer() function as the address to parameter. The msg.sender argument is passed to the _transfer() function as the address from parameter.

msg.sender / from
is transferring part, or all, of their own share of the total amount of ether held in the contract
TO
recipient / to

@Bitborn

Supposing I approve someone to spend my funds. That person can then use the transferFrom to send my funds to whomever he wants correct?

The Bank contract used in the course/assignments doesn’t allow an address to approve another address to perform any transactions on its behalf. Our use of msg.sender in the code ensures that…

1. The caller of the deposit() function can only transfer ether from their external address balance to the contract, where it is recorded in the mapping as their own share of the total contract balance:   balance[msg.sender] += msg.value

2. The caller of the withdraw() function can only transfer ether out of the contract to their own external address balance:   msg.sender.transfer(amount)
   … and that this reduction in the total contract ether balance can only be recorded as a reduction in that same caller’s own share of the total contract balance:   balance[msg.sender] -= amount

3. The caller of the transfer() function can only reallocate part, or all, of their own share of the total amount of ether held in the contract:
   balance[from] -= amount
from is msg.sender passed to _transfer() helper function from public transfer() function
   … so that ownership of this share of the total contract ether balance (expressed as an amount) is transferred to an address of their choosing (input as the recipient argument):
   balance[to] += amount
to is recipient passed to _transfer() helper function from public transfer() function

I hope that helps to clarify things some more. But if I’ve misunderstood what you mean by the transferFrom() function, then post your full Bank contract code and I’ll take another look. Let me know if anything I’ve explained is unclear, or if you have any further questions