Inheritance & External Contracts - Discussion

On the whole, you have some good answers here @Andre_Mark :ok_hand:

Q1 & Q3  :white_check_mark:

Q2

Just to confirm… as with functions, state variables also need to have public or internal visibility to be inherited by derived contracts. State variables marked private are not available.

With this clarified, the above part of your answer is correct, but you don’t need to add “to an extent”.

But this isn’t correct…
The type of inheritance structure determines which contracts within that structure share a parent/child relationship. If there is just one parent/child in the inheritance structure it is single inheritance.
If there are two parent/child relationships then this could be:

  • multi-level inheritance, if   contract B is A   and   contract C is B
  • hierarchical inheritance, if   contract B is A   and   contract C is A
  • multiple inheritance, if   contract C is A, B

But whichever of the above (or multiple other possible structures) occurs, it will be made up of either one or more than one parent/child relationship, and the derived contract in each of these “pairs” will always inherit from its specific parent/base contract according to exactly the same rules of inheritance.

Let me know if you have any questions :slight_smile:

1 Like

Jon, thank you sooo much for this explanation, I am so impressed by your thorough response! Now I understand, the “amount” parameter just tells the Bank owner how much Paul paid Peter, and the {value: 1 gwei} is what the Bank owner must pay the government to allow for Paul and Peter to transfer funds using the Bank’s platform. My only observation about that would be that the Govermnent contract itself doesn’t have any programmed cost because the function collectTax() is open ended and up to the Bank to determine how much tax it must pay. If I were the government I would have a parameter tax = 1 gwei, and all the bank has to do is pass it to their internal function. Does “{value: tax}” would work? Would it accept variables or must it be only wei, gwei or ether inside? Thanks!!

1 Like

Hi,
So I don’t know if anyone else experiences this, but I have been testing the external contracts, and after I have executed the transfer between accounts, I go to the government contract and enter zero into the getTransaction and receive the below error message;

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

invalid opcode

The execution might have thrown.

Debug the transaction to get more information.

Any advice?

Hi @Andre_Mark,

If you post your full code for both contracts (Bank and Government), I’ll take a look for you.

Hey Marcos,

I’m glad you found my explanation helpful :slight_smile:

Yes, you are absolutely right. Our example is very simplistic, and far from perfect.

And yes, that would work, e.g.

Add the following code to GOVERNMENT …

/* A tax state variable, which can be set and modified by an
   authorized address e.g. the contract owner.
   Also needs a constructor and onlyOwner modifier, which could be
   inherited from Ownable. */

uint tax;

function setTax(uint _tax) external onlyOwner {
    tax = _tax;
}

// Can be called by Bank to get the amount of tax to pay
function getTax() external view returns(uint _tax) {
    return tax;
}

/* In addTransaction() include the following require statement
   to ensure the correct amount of tax is paid per transaction */
require(msg.value == tax, "Incorrect tax payment");

Add the following code to BANK

// Include getTax() header in the Government interface
function getTax() external view returns(uint _tax);

/* A tax state variable, which can be set and modified by the contract
   owner to the amount of tax authorized for payment on each transaction */

uint public tax;

function setTax(uint _tax) external onlyOwner {
    tax = _tax;
}

// Called from within transfer()
function _government(address sender, address recipient, uint amount) private {
    uint _tax = governmentInstance.getTax();

    // Checks tax amount requested by Government has been authorised by Bank
    require(_tax == tax, "Unauthorised tax payment requested");
    
    // Moved to here from transfer()
    governmentInstance.addTransaction{value: _tax}(sender, recipient, amount);
}

Have an experiment yourself… there will many possible alternatives which could be implemented, and the one I’ve outlined above is by no means meant to be the best — it’s just the one I’ve come with as an example to show you what’s possible.

Let me know if you have any further questions :slight_smile:

pragma solidity 0.7.5;


import "./Destroyable.sol";

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

contract bank is Destroyable {
    
    governmentInterface governmentInstance = governmentInterface (0x5B38Da6a701c568545dCfcB03FcB875f56beddC4); 
    
    mapping (address => uint) balance;
    
    event deposited(uint amount, address depositedTo);
  
    function deposit () public payable returns (uint) {
        balance[msg.sender] += msg.value;
        emit deposited(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function withdraw (uint amount) public returns (uint) {
        require(balance[msg.sender] >= amount, "Insuffcient funds");
        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 insufficient");
        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; 
        
    }`Preformatted text`
pragma solidity 0.7.5;

contract Government {
    
    struct Transaction {
        address from;
        address to;
        uint amount;
        uint transID;
    }
    
    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);
    }
    
    
}

You’ve missed out the external function call to governmentInstance.addTransaction()

You’ve created an instance of the Government contract in Bank with this line:

… but you’re not using it. Unless you make an external call to addTransaction() from within your transfer() function, and pass the details of each transfer to this function, then the transfers will execute successfully in Bank, but the addTransaction function will not be executed, and will not push any data to the Transaction Log array. Therefore, your array is still empty, and that must be why you get an error when trying to retrieve data at index 0, because it doesn’t exist.

If you’re not sure how to code the external function call, then watch the video again first, but then just let me know if you have any further questions.

1 Like

Thanks, I feel a bit lazy actually, yes, watched the video again, it was a simple fix, I changed all the characters to lowercase in this instance, lesson learned, I’ll just watch the videos again in future.

Andre

1 Like

You’re the best, Jon! Thankss

1 Like

Hello,
So I have been trying to add value functionality to the external call assignment and came across an issue I was hoping someone could help me with.

I went through both the bank contract and the government contract and made the appropriate functions payable. Then I set up the bank contract so that one address had deposited 2 ETH into the contract and another address had deposited 1 ETH. When I execute the transfer function to send 1 ETH from the address that has 2 to the address that has 1, the transaction info is successfully sent to the government contract along with a payment of 1 ether. However, after performing this function, when I check the balance of the bank contract, it says the overall balance of the contract is only 2 ether, even though when I check the individual balances of the two addresses that used the contract, one still has 2 ETH and the other 1 ETH.

I am curious where the 1 ether that made it to the government contract came from and why the getBalance function for the bank contract now returns an incorrect balance after the transfer is completed. Any help would be greatly appreciated and I have pasted the relevant code below, thank you!

Bank.sol
Value Calls 1

Government.sol
Value Calls 2

Hey @Ben17, hope you are well.

Please share your code in the following way so we can replicate your issue and help you solve it :face_with_monocle:

Carlos Z

My apologies, thank you for letting me know the proper format. Here is the entire code.

Bank.sol

pragma solidity 0.7.5;

import "./Destroyable.sol";

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

contract Bank is Destroyable {
    
    GovernmentInterface governmentTxReport = GovernmentInterface(0xd9145CCE52D386f254917e481eB44e9943F39138);
    
    mapping (address => uint) balance;
    
    event balanceAdded(uint amount, address depositedTo);
    
    function deposit() public payable returns (uint){
        balance[msg.sender] += msg.value;
        emit balanceAdded(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function withdraw(uint amount) public returns (uint){
        require(amount <= balance[msg.sender]);
        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 payable {
        require(balance[msg.sender] >= amount, "Balance not sufficient.");
        require(msg.sender != recipient, "You can't transfer money to yourself.");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        governmentTxReport.addTransaction{value: 1 ether}(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;
    }
    
    function getContractBalance() public view returns(uint){
        return address(this).balance;
    }
}

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 payable {
        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);
    }
    
    function getBalance() public view returns(uint){
        return address(this).balance;
    }
}

Ownable_1.sol

pragma solidity 0.7.5;

contract Ownable_1 {

address payable internal owner; 
    
    constructor(){
        owner = msg.sender;
    }
    
    modifier onlyOwner{
        require(msg.sender == owner, "Only the Owner can Terminate this Contract.");
        _; // run the function. 
    }
    
}

Destroyable.sol

pragma solidity 0.7.5;

import "./Ownable_1.sol";

contract Destroyable is Ownable_1 {
    
    function close() public onlyOwner { 
        selfdestruct(owner); 
    }
 
}

For my bank.sol, I just changed the header to:

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

contract Bank is Ownable, Destroyable {

For my Destroyable.sol, I wrote:

import "./Ownable.sol";

contract Destroyable is Ownable {
    address payable owner;

    function destroy() public onlyOwner {
        owner = payable(msg.sender);
        selfdestruct(owner);
        }
    }

Could you tell me:

  1. Why does the address in Destroyable.sol need to be payable?
  2. Does the destroy() function need to be public?

The problem came from this function, when adding a transaction to the government contract, you are also sending funds to it (value: 1 ether)

    function transfer(address recipient, uint amount) public payable {
        require(balance[msg.sender] >= amount, "Balance not sufficient.");
        require(msg.sender != recipient, "You can't transfer money to yourself.");
        
        uint previousSenderBalance = balance[msg.sender];
        
        _transfer(msg.sender, recipient, amount);
        
        governmentTxReport.addTransaction{value: 1 ether}(msg.sender, recipient, amount);
        
        assert(balance[msg.sender] == previousSenderBalance - amount);
    }
    

This line exactly governmentTxReport.addTransaction{value: 1 ether}(msg.sender, recipient, amount);

Thats why when depositing 2 ethers, transfering them to another account, 1 ether goes to the government contract (hard coded) and the other will go to the balance of the account.

Carlos Z

Thank you for taking a look at the issue. I’ll try to figure out how to make the transfer work better then. Just a clarifying question though:
I’m still confused why the getBalance function for the Bank.sol no longer returns the correct balance after the transaction. I thought that getBalance simply added up the sum of all the accounts on the smart contract and gave a total. After I perform a transfer, and 1 ETH goes to Government.sol, the getBalance function for Bank.sol says that there are only 2 ETH total left in the Bank.sol contract, but after individually checking the two addresses I used for the test, there are clearly still 3 ETH total, 1 in one account, and 2 in another. Just wondering how this is possible and what numbers the getBalance function is using to arrive at its total.
Thanks again for your assistance :slight_smile:

1 Like

Hi. Not sure why, but I am getting this error. Could someone tell me what they think is going on here?

Screen Shot 2021-08-05 at 9.29.17 PM

2 Likes

Can anyone tell me what might be wrong with these 2 line? They are both throwing errors in the Bank Contract for me…

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

governmentInstance.addTransaction{value: 1 Ether}(msg.sender, recipient, amount);

1 Like

The problem come from the way you manage the funds, when doing a transfer, it will discount the amount from the balance[msg.sender], but the contract will pay 1 ether from its funds to governmentTxReport.addTransaction, instead of discounting it from msg.sender. Thats why, contract balance have less ethers than what the balance mapping shows on each account.

Carlos Z

Hey @alexhupe, @Bitborn, hope you guys are ok.

Please share your code in the following way so i can check why those error are being triggered :face_with_monocle:

Carlos Z

Oh ok, got it, thank you for the clarification!