Inheritance & External Contracts - Discussion

[david.maluenda]

I’m playing around with the Government contract but I’m not able to add the event log successfully,
Where should the “event” & “emit” string be added in the government Code.
Code below:

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

}

[jon_m]

Hey @david.maluenda,

The idea isn’t to emit an event, but to call getTransaction() on the Government contract, after a transfer has been executed (i.e. the transfer function called) in Bank.

You need to call getTransaction() with the index of the transaction you want to view. So the first transfer made in Bank will be logged in the Government contract’s transaction log at index 0, the second transfer at index 1 etc.

For the external function call   governmentInstance.addTransaction()   to work you will have deployed both the Government contract and Bank, and so you’ll need to ensure that you’ve expanded the Government contract, as well as Bank, in the Deploy & Run Transactions panel in Remix, in order to be able to add the index argument for getTransaction and then click its blue button to retrieve the data from the transaction log.

Let me know if that makes sense, or if you have any further questions about how to get this working practically

[jon_m]

Hi @david.maluenda,

Following on from my previous post …

You should already have an event emitted for each transfer transaction in Bank. This should be emitted at the end of the transfer() function, after the transfer has been performed within Bank and the external Government function (addTransaction) has been executed successfully (adding the transfer data to the Government transaction log).

You could potentially add an additional event emitted by the Government contract as additional confirmation of the transaction data added to the transaction log array. But in terms of the transfer amount, and its sender and recipient addresses, this data is already contained in the event emitted by Bank. If the Government’s addTransaction() function failed to add the data to the transaction log, then the transfer() function in Bank would also fail to finish executing, and so its transfer event wouldn’t be emitted anyway. However, emitting the txId (the transaction’s index position in the array) could be useful in order to call the getTransaction() function and retrieve a specific transaction’s data afterwards. You could add an event declaration to the top of the Government contract, and the corresponding emit statement to the end of the addTransaction() function e.g.

pragma solidity 0.7.5;

contract Government {

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

   event GovTxLog(uint txId);  // ADD event declaration

   Transaction[] transactionLog;

   function addTransaction(address _from, address _to, uint _amount) external {
      /* Add a local variable to save transactionLog.length before it increases,
      to ensure the txId emitted in the event is the same index number */
      uint txId = transactionLog.length;  // ADD
      // Reference variable txId instead of expression transactionLog.length
      transactionLog.push(Transaction(_from, _to, _amount, txId));
      emit GovTxLog(txId);  // ADD emit statement
   }

   // etc.
}

Both events will now be emitted (one from each contract) when the transfer() function is executed. There is only one transaction, though, so both logged events will appear together in the logs of the same transaction receipt generated in the Remix console. However, the event data emitted from the external Government contract is not displayed in the same reader-friendly format as that emitted from Bank — although, if you know what you’re looking for, you can see that both events have been logged, as follows …

[
   {  //** Logged GovTxLog event data **//
      "from": "0x1bB...7B0b",  // Smart contract address (Government) 
      "data": "0x000...0001",  /* Emitted event argument(s) (except if indexed)
       txId => 1  (2nd tx logged in array) integer expressed as hexadecimal */
      "topics": ["0x964...aa26"]  /* 1st topic => Event Signature (hash of
       event name and its parameter types). Up to 3 more topics => one for
       each indexed argument (but there aren't any in this example) */
   },

   {  //** logged Transfer event data **//
      "from": "0xdda...482d",   // Smart contract address (Bank)
      "topic": "0xddf...b3ef",  // Event Signature
      "event": "Transfer",      // Event name
      "args": {  // Logged data, according to the defined event arguments
                 // Same data given twice (i) by index (ii) by argument name 
         "0": "0x5B3...ddC4",       // Sender's address
         "1": "0xAb8...5cb2",        // Recipient's address
         "2": "2000000000000000000",  // Amount transferred (always in wei)
         "from": "0x5B3...ddC4",       // Sender's address (repeated)
         "to": "0xAb8...5cb2",          // Recipient's address (repeated)
         "amount": "2000000000000000000" // Amount transferred (repeated)
      }
   }
]

Just let me know if you have any questions

[david.maluenda]

Hello Peeps, need help from you guys!

When deploying my contract I get the following message:

Cant really see what the problem is, doing the exact same thing as Filip is doing in the video.

My code:

---

pragma solidity 0.7.5;

import “./Ownable.sol”;

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

contract Bank is Ownable {

GovernmentInterface governmentInstance = GovernmentInterface(0x3643b7a9F6338115159a4D3a2cc678C99aD657aa);



mapping(address => uint)balance;



event depositDone(uint amount, address indexed depositTo);
event transferDone(address from, address to, uint amount);



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


function withdraw(uint amount) public returns(uint) {
    require(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, "Dont have enough money");
    require(msg.sender != recipient, "Dont send money to yourself");
    
    uint balanceBeforeSending = balance[msg.sender];
    
    _transfer(msg.sender, recipient, amount);
    
    governmentInstance.addTransaction(msg.sender, recipient, amount);
    
    assert(balance[msg.sender] == balanceBeforeSending - amount);
}


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

}

---

[jon_m]

Hi @david.maluenda,

I can’t see anything in your Bank contract that could be causing the transfer() function to revert. The most common cause of this issue is not changing the Government contract’s address, which is assigned to your governmentInstance. Have a look at this post, which explains the issue and how to resolve it. It also explains why the error message you got isn’t very helpful.

Let me know if that doesn’t work, and I’ll take a look at your Government contract as well.

[jon_m]

Having looked at your Bank contract, I’ve also noticed a couple of other things (unrelated to the error message) …

(1) Your withdraw function is missing an essential line of code …

If you deploy your contract, make a deposit, and then call your withdraw function as it is, you will notice that the caller’s external wallet address receives the requested withdrawal amount, but their balance in the mapping is not reduced! This means that they can make repeat withdrawals — each one no greater than their initial balance, which isn’t being reduced — up to the total amount of ether held in the contract (the sum of all of the individual account holders’ balances). So, I’m sure you can see why this is a very serious bug!

msg.sender.transfer(amount) transfers ether from the contract address balance to the caller’s external wallet address, but it doesn’t adjust the individual user balances in the mapping. 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.

Notice, as well, that we’ve made similar adjustments to both the sender’s and the recipient’s balances in the mapping in the _transfer() function: the sender’s balance is reduced by the same amount that the recipient’s balance is increased by.

Once you’ve modified your withdraw function for this, have a look at this post which explains the importance of the order of the statements within the function body.

(2) Your transfer event and corresponding emit statement are both well coded, and the emit statement will log appropriate data when the transfer() function is successfully executed.

A couple of comments…

• It’s better to emit the event at the very end of the function which completes the transaction i.e. transfer() not _transfer().
  _transfer() is what we call a helper function .
  Another important factor here is that if you emit an event in a helper function, you are restricting your ability to reuse that helper function. It will be more reuseable if we can call it from another function when we may not want to emit the same event — either a different one, or no event at all. That way we can reuse the helper function whenever we just want its functionality (i.e. the operations, or computation, that it performs).
• In general an emit statement is probably better placed after an assert statement.

---

You also seem to have missed out the Inheritance Assignment. I would definitely go back and complete that before you finish this course and move on to 201, just to make sure you are comfortable with how inheritance works and how to code for it.

Let me know if anything is unclear, or if you have any questions about any of these additional points

[david.maluenda]

Thanks for your answer mate, forgot about the assignment will go back and fix it.

[Shadowstep2003]

I’m not entirely sure what I’m doing wrong here with this value call. I keep getting an error on the Transfer function from Bank Contract:

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.

Can someone offer some advice?

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() external view returns(uint) {
        return address(this).balance;  // returns the balance of this contracts address built-in function
    }
}

pragma solidity 0.7.5;

import "./Destroyable.sol";

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

contract Bank is Destroyable {

 GovernmentInterface governmentInstance = GovernmentInterface(0xa2a7b718Af3CD7F18354Ac5E02235ea6C035BD57);    

 mapping (address => uint) balance; 
 
 event depositDone(uint amount, address depositedTo);
 event transferSent(uint amount, address sentFrom, address sentTo);
 
 
 
 function deposit() public payable returns (uint) {
     balance[msg.sender] += msg.value;
     emit depositDone(msg.value, msg.sender);
     return balance[msg.sender];
 }
 
 function withdraw(uint amount) public onlyOwner {
     //msg.sender is an address
     require(balance[msg.sender] >= amount, "Insufficient amount to transfer");
     balance[msg.sender] -= amount;
     msg.sender.transfer(amount);
 }
 
 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, "Don't transfer money to yourself");
     
     uint previousSenderBalance = balance[msg.sender];
     
     _transfer(msg.sender, recipient, amount);
     governmentInstance.addTransaction{value: 1 ether}(msg.sender, recipient, amount);
     
     emit transferSent(amount, msg.sender, recipient);
     
     assert(balance[msg.sender] == previousSenderBalance - amount);
     
 }
 
 function _transfer(address from, address to, uint amount) private {
     balance[from] -= amount;
     balance[to] += amount;
 }
 
 function getThisBalance() public view returns(uint) {
        return address(this).balance;  // returns the balance of this contracts address built-in function
    }
 
 
}

[jon_m]

Hi @Shadowstep2003,

I’ve deployed and tested your contracts and your transfer() function executes successfully, including the external function call and value transfer to Government.

The most common cause of the transfer() function reverting, with the error message you’ve received, is not changing the Government contract’s address, which is assigned to your governmentInstance. Have a look at this post, which explains the issue and how to resolve it. It also explains why the error message you got isn’t very helpful.

A couple of other observations about your Bank contract …

(1) Notice that the address calling the transfer() function is not sending ether from their external address to the Bank contract (as they do when calling the deposit function). The transfer performed is an internal reallocation of funds between two individual users. A function only needs to be marked payable when it needs to receive ether. So, unlike the deposit() function, we don’t want to mark transfer() as payable in its function header.

Within the transfer() function, the external function call to addTransaction() does involve ether being transferred from Bank to the Government contract (as a tax on the transfer transaction, or as a fee for recording it in the Government transaction log). But this value transfer only requires the receiving function (addTransaction in Government) to be marked payable, and not the sending function (transfer in Bank).

Marking the transfer() function as payable doesn’t prevent the code from compiling, or the function from executing, but it is bad practice and poor risk management not to restrict a function to non-payable if it doesn’t need to receive ether, because if someone sends ether by mistake, this amount will still be transferred to the contract. For example, if the caller sent an ether value to the transfer() function by mistake (as well as calling it with a transfer amount and a recipient address) then this ether would be added to the Bank contract balance and potentially lost to the user, because such a deposit wouldn’t also be added to the user’s individual account balance in the mapping (only the transfer amount is deducted from their balance). However, the function is much securer if we leave it as non-payable, because then, if the caller made the same mistake, the function would revert, and the ether would not be sent.

(2) You’ve applied the onlyOwner modifier to the withdraw() function, but this means that only the contract owner can withdraw funds up to their own individual balance. But the contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so it only seems fair that all users should be able to withdraw their funds as well, don’t you think?

Let me know if you have any questions about these points, or if you’re still having problems calling the transfer() function.

[Ruben99]

1. What is the base contract?

   The base contract is the “parent” contract from which we could derive their properties into child
   contracts.

2. Which functions are available for derived contracts?

   We have access to all the functions that are declared public or internal.

3. What is hierarchical inheritance?

   It’s a type of inheritance in which we can have multiple derived contracts from the parent.

[josejalapeno]

1. base contract is the parent contract
2. polymorphism
3. hierarchical inheritance is like single inheritance, where multiple contracts are derived to a single contract

[jon_m]

Hi @josejalapeno,

Q1

Q2 Which functions are available for derived contracts?

@josejalapeno

2. polymorphism

No … it is the visibility of a function in the base contract, which determines if it is inherited by the derived contract or not.

Functions in the base contract with public or internal visibility are inherited, but those with private or external visibility are not available for derived contracts.

---

Q3 What is hierarchical inheritance?

@josejalapeno

3. hierarchical inheritance is like single inheritance, where multiple contracts are derived to a single contract

I think you’ve understood this. But just to confirm …

Hierarchical inheritance is “where multiple child contracts are derived from a single base contract”.
(Single inheritance is where a single child contract is derived from a single base contract.)

Let me know if you have any questions

[josejalapeno]

thanks for clarifying the number 2 question and yes I understand the number 3 question.

[michael356]

1. What is the base contract?
   the contract has a parent known as the derived class and the parent contract is known as a base contract

2. Which functions are available for derived contracts?

all public and internal scoped
functions and state variables are available to derived contracts。

3. What is hierarchical inheritance?

a single contract acts as a base contract for multiple derived contracts.

[jon_m]

Nice answers @michael356

@michael356

1. What is the base contract?

This part of your answer is correct …

@michael356

… the parent contract is known as a base contract

But this part is actually an error in the article …

@michael356

the contract has a parent known as the derived class …

The parent contract (or base contract) can also be called the base class. These are all alternative terms for the same thing.

The child contract (or derived contract) can also be called the derived class. These are also three alternative terms for the same thing.

Let me know if you have any questions.

[ecbwong]

1. The “base contract” refers to the parent contract in a parent-child relationship.

2. All public and internal scoped functions and state variables are available to the derived contracts. And

3. A hierarchical inheritance is where a single contract forms the base for multiple derived contracts.

[_Vincent]

1. What is the base contract?
   If there is a multiple, hierarchical, level inheritance between two contracts or more, base contract define the parent contract (top-level).

2. Which functions are available for derived contracts?
   Public and internal functions are available for derived contracts.

3. What is hierarchical inheritance?
   This is when we have two or more child contract for a derived class (the base contract).

[jon_m]

Nice answers @_Vincent

A couple of clarifications …

@_Vincent

What is the base contract?
If there is a multiple, hierarchical, level inheritance between two contracts or more, base contract define the parent contract (top-level).

Just to be clear, a base contract is any parent contract that is inherited by a derived contract. Within an inheritance structure there can be more than one base contract e.g.

// Multi-level inheritance structure
contract A { ... }
contract B is A { ... }
contract C is B { ... }

In this multi-level inheritance structure:
C is a derived contract
A is a base contract… but not the only base contract …
B is both a derived contract (of A), and a base contract (to C)

Basically…
parent contract = base contract
child contract = derived contract
They are just alternative terms for the same thing.

@_Vincent

What is hierarchical inheritance?
This is when we have two or more child contract for a derived class (the base contract).

This is correct apart from the bit I’ve highlighed in bold. There is actually an error in the article where it says that a parent contract can also be called the derived class. The correct terminology is as follows …

The parent contract (or base contract ) can also be called the base class. These are all alternative terms for the same thing.

The child contract (or derived contract ) can also be called the derived class. These are also three alternative terms for the same thing.

So, hierarchical inheritance is where two or more child contracts are derived from the same, single base contract (base class).

Let me know if you have any questions

[_Vincent]

Thank you so much for taking time for this detailed answer.
And thank you for the correction on last question ; here is an imoprtant information to highlight in official video, before people read the linked blog.

[_Vincent]

Hey everyone,
a question regarding the course video :
We need to compile the lowest level in order to compile all contracts (in the course instance, it gives to compile the Bank contract for compiling Bank and Ownable contracts both).

But, in Remix, what if there are 50 derived contracts from Owner base contract. We have to compile the 50 files one by one in Remix ?

Thanks,
Vincent.