Inheritance & External Contracts - Discussion

[jon_m]

@Al86

I need to practice coding on more than 3 hours of sleep

That would be a good idea, Al

[BShehaj]

1. What is the base contract?
   The contract that is inherited, therefore the parent contract.
2. Which functions are available for derived contracts?
   The functions that have public or internal visibility.
3. What is hierarchical inheritance?
   It is when a single contract acts as a base contract for some derived contracts.

[jeanphi]

Hi there,
I don’t understand this concept of ‘contract balance’ that as @filip said we can always check with address(this).balance
So far we have worked on user account balance only, and we have created a specific mapping variable for this.
So what is a contract balance ?
Thanks for your help.

[jon_m]

Hi @jeanphi,

When users call the deposit() function, ether is transferred from their external wallet addresses to the contract address. The contract address balance is therefore the total amount of ether belonging to all account holders. The balances in the mapping perform an internal accounting role, and keep track of each user’s share of the total pooled funds. Without them there would be no way of knowing what each individual user’s entitlement was. The ether itself is held by the contract address, and not in the mapping.

When a user calls the withdraw() function, ether is transferred from the contract address to the user’s external wallet address. First of all we perform the internal accounting adjustment to record the reduction in the user’s share of the total pooled funds:

balance[msg.sender] -= amount;

Then we perform the actual transfer of ether from the contract address balance to the user’s external wallet address, using:

<address payable>.transfer(uint256 amount);

When a user calls the transfer() function, no ether is entering or leaving the contract. The total contract address balance doesn’t change. It is purely an internal transfer between users which results in a decrease in the sender’s share of the total contract balance, and an increase, by an equal amount, in the recipient’s share of the total contract balance. It is therefore only necessary to adjust each user’s individual balance in the mapping (two equal and opposite internal accounting adjustments).

You can see a user’s external wallet address balance increase and decrease as ether is either withdrawn from, or transferred to the Bank contract. We can check the total amount of ether held in the contract (the contract address balance) by creating a getter and returning address(this).balance  e.g.

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

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

[jeanphi]

@jon_m

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

Thanks @jon_m for replying so quickly!
I think I understand better these concepts now but can you confirm 2 more things for me:

1. Why Filip’s solution here https://github.com/filipmartinsson/solidity-0.7.5/blob/main/inheritance-assignment/Bank.sol shows a withdraw function that is not actually withdrawing any amount from the user account internal balances mapping?
2. Why is deposit payable but not withdray? Both need a ETH value as an input that is impacting the contract total balance?
   Thank you!
   JP

[jon_m]

Hi @jeanphi,

@jeanphi

Why Filip’s solution here https://github.com/filipmartinsson/solidity-0.7.5/blob/main/inheritance-assignment/Bank.sol shows a withdraw function that is not actually withdrawing any amount from the user account internal balances mapping?

This is an error, the withdraw function in the solution should include the deduction of the withdrawal amount from the user’s balance in the mapping, as follows:

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

In addition, access to the withdraw function shouldn’t be restricted by the onlyOwner modifier. If you think about it, this doesn’t make any sense, because otherwise all users, except for the contract owner, will not be able to withdraw any of the ether they have deposited!

The solution code for the Transfer Assignment is actually correct for both of these points:
https://github.com/filipmartinsson/solidity-0.7.5/blob/main/transfer-assignment-solution.sol

---

@jeanphi

Why is deposit payable but not withdray? Both need a ETH value as an input that is impacting the contract total balance?

A function only needs to be marked payable when it receives ether from an external address to add to the contract address balance. It might help to think of payable 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 a track of their share of the total funds held in the contract.

In contrast, the withdraw function is deducting ether from the contract balance and sending 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(uint256 amount);

We should not mark the withdraw function as payable, because it does not need to receive ether from an external address in order to update the contract balance. If we made it payable, then, if the caller sent 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, because there is no accounting adjustment made to the mapping in the withdraw function 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 made the same mistake, the function would revert, and the ether would not be sent.

Just let me know if you have any further questions

[jeanphi]

Perfect! This all make sense! Thank you for the precision in your reply!

[CryptoDiddy]

1. What is the base contract?
The base contract is the parent contract

2. Which functions are available for derived contracts?
Public
Internal scoped
State variables

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

[jon_m]

Nice answers @CryptoDiddy

Just one thing…

@CryptoDiddy

2. Which functions are available for derived contracts?
Public
Internal scoped
State variables

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. On the other hand, all modifiers and all events in the parent contract are inheried.

Just let me know if you have any questions

[Samm]

I hope I am in the right place and that I have the right idea on Ownable/Destroyable inheritance. Here is what I came up with:

================= Ownable.sol =================
pragma solidity 0.8.4;

contract Ownable {
address public owner;

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

constructor() {
    owner = msg.sender;
}

}
================ Destroyable.sol ======================
pragma solidity 0.8.4;

import “./Ownable.sol”;

contract Destroyable is Ownable {

event contractClosed(uint closeBalance, address indexed closeAddress);

function destroyContract() onlyOwner public { 
    uint contractBalance = address (this).balance;
    selfdestruct(payable (owner)); 
    assert(address (this).balance == 0);
    emit contractClosed(contractBalance, owner);
}

}================ helloworld.sol ===============
pragma solidity 0.8.4;

import “./Ownable.sol”;
import “./Destroyable.sol”;

contract Bank is Ownable, Destroyable {
…
}

I can now call destroyContract() from Remix, but only from the original
deployer of the contract, not from any other address, so that appears correct,
and I DO, in fact get my balance back, plus what was in the other address.

I am wondering, though, where do I see the logs again?
I am hoping to see the contract balance that gets sent to the
contract owner when it is destroyed via the event called in Destroyable.sol.

Thanks!

[Samm]

Hi, FrankB,
I believe that modifiers are similar to compiler directives in that the modifier code is actually inserted into your function at compile time, therefor it is not really “referenced” publicly or privately.

Then again, I may be wrong…

[Samm]

Well, my Bank contract was working well until I added the Government Interface and broke it!

First question: Why would the addTransaction function be payable? Does it need to be or was that an example?

Second question: Why am I getting this error when I call addTransaction()?
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.

In Bank contract:

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

in transfer() function:
governmentInstance.addTransaction{value: 1 wei}(msg.sender, recipient, amount);

In Government contract:
function addTransaction(address _from, address _to, uint _amount) external payable {
transactionLog.push( Transaction(_from, _to, _amount, transactionLog.length) );
}

Thanks.

Oops. Forgot this line in Bank Contract:

IGovernment governmentInstance = IGovernment(0xdD870f…7392148);

[Joey]

I have the same error after i made the function in government payable and added value to the instance.

[Samm]

Somehow I got mine to work. Did you?
I just kept testing the Government contract stand-alone until it worked, then tried it again from the Bank contract. Not sure why it wasn’t working in the first place!

[jon_m]

Hi @Samm,

Firstly, apologies for the delay in giving you some feedback on this, and in answering your specific question about the event logs.

@Samm

I can now call destroyContract() from Remix, but only from the original
deployer of the contract, not from any other address, so that appears correct

Yes… your solution achieves this objective nicely

@Samm

and I DO, in fact get my balance back, plus what was in the other address.

Yes… your solution also handles this appropriately. Well done for realising that you needed to make the owner address payable , so that selfdestruct() is called with a payable address and, therefore, any remaining ether in the contract can be transferred to the owner when selfdestruct() is triggered The remaining contract balance is transferred, and so it’s also correct that the contract owner will receive the sum of all of the individual users’ remaining account balances (as well as their own).

Your coding of the inheritance structure is sound. Note, that 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.

---

@Samm

I am wondering, though, where do I see the logs again?
I am hoping to see the contract balance that gets sent to the
contract owner when it is destroyed via the event called in Destroyable.sol.

Below are some excerpts from discussions I’ve had with a couple of other students about the issues related to emitting an event for selfdestruct() …

@jon_m

@Anvar_Xadja

I was trying to put an event for destroy function, but when this function is actually exectued, I have not seen any logs. Is there any specific reason for that?

If the emit statement is placed after the call to selfdestruct(), the event won’t be emitted because the contract has been destroyed once selfdestruct() has executed, and so the emit statement is never reached. You could fix this by placing the emit statement before the selfdestruct() function call, but I don’t think this is wise, because we would be emitting the event before the actual event (contract destruction) has ocurred. On the other hand, if selfdestruct() failed, then the function would revert, including the emitted event, but I would still be reluctant to place the emit statement first. This needs some further investigation…

So in your destroyContract function, both the emit statement and the assert statement are unreachable and will not be executed.

@jon_m

You will notice that one of the disadvantages of selfdestruct() is that successful calls can still be made to the destroyed contract’s functions! This is a serious problem if a user isn’t informed that the contract has been destroyed and unwittingly sends ether to the contract, because it will be lost.

@jon_m

… it’s also worth bearing in mind that emitting an event doesn’t necessarily mean that users will see that infomation, and so doesn’t actually prevent them from subsequently calling the destroyed contract’s functions, incurring a gas cost, and losing any ether sent. 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. This is also why, in practice, other types of security mechanisms, such as proxy contracts, are implemented to deal with these kinds of emergency situations. You will learn more about the practicalities of this if you go on to take the Smart Contract Security course.

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

[Samm]

Thanks so much for the explanation. Especially “…inherit Destroyable only, because it will inherit Ownable implicitly via Destroyable.” and “… the event won’t be emitted because the contract has been destroyed once selfdestruct() has executed, and so the emit statement is never reached…”

These were news to me and very informative. I’m anxious to learn more about proxy contracts!

[jon_m]

Glad it was informative
I think you’ll definitely enjoy the Smart Contract Security course…

[jon_m]

Hey @Samm,

Once again, apologies for the delay in giving you some answers on this.

@Samm

First question: Why would the addTransaction function be payable? Does it need to be or was that an example?

It doesn’t have to be. Our example is based on a contractual relationship between a government and a bank. The idea is that the government needs to collect some kind of tax on each banking transaction that is a transfer. To keep things simple in our example, the government is collecting a fixed amount (1 wei) on each transfer. This amount will be automatically transferred from the Bank contract balance to the Government contract balance on each successfully executed transfer transaction. For record keeping purposes, the relevant data for each transfer is also sent from Bank to Government and stored in the government’s transaction log (an array).

Instead of being a tax, another way to interpret this payment could be as payment for some kind of record keeping service i.e. the Bank contract pays the Government contract for storing all of its transaction data.

However we interpret this payment from one contract to the other, it demonstrates (albeit in an overly simplistic way) how we can program automatic interactions between individual contracts for the purposes of both data and value transfer. Hopefully, this example shows the potential for these sorts of programmed interactions to be applied to many different use cases.

But returning to your original question… we could equally exclude the transfer of value from the addTransaction function, if we want it to provide a purely record keeping role at no extra charge (except of course the cost of gas consumption, which in any case is paid to the miners and not to the external contract).

If no value is transferred, then the addTransaction function should not be marked payable, in both Government and the Government interface in Bank.

---

@Samm

Second question: Why am I getting this error when I call addTransaction()?
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.

You should be deploying your Government contract before deploying Bank. If you redeploy Government at any stage, 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:

@Samm

IGovernment governmentInstance = IGovernment(0xdD870f…7392148);

If you forgot to replace the previous address of your external contract for the new one, then this will have caused the transaction to revert when you called the transfer function, and will have given you the error message you got. The problem is that the error message you got was just a default one, and so that’s why it wasn’t helpful in pinpointing the error. Your error most probably wasn’t anything to do with the function not being marked payable, or the value exceeding the current balance.

This is easily done, and catches most of us out the first time we make that mistake

Adding an error message to your require() statements, is always a good idea, because if the revert is caused by one of them, the message you added will appear as part of the error message you get in the Remix console. If one of your own error messages doesn’t appear, you know that the transaction reverted because of something else.

Let me know if you think that was the cause of the problem or not. But I’m glad to see that you got it to work in the end

@Samm

Somehow I got mine to work. Did you?
I just kept testing the Government contract stand-alone until it worked, then tried it again from the Bank contract. Not sure why it wasn’t working in the first place!

[jon_m]

Hey @Joey,

I’ve replied to @Samm regarding this issue. Have a look to see if this solves the problem:

@jon_m

@Samm

Second question: Why am I getting this error when I call addTransaction()?
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.

You should be deploying your Government contract before deploying Bank. If you redeploy Government at any stage, 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:

@Samm

IGovernment governmentInstance = IGovernment(0xdD870f…7392148);

If you forgot to replace the previous address of your external contract for the new one, then this will have caused the transaction to revert when you called the transfer function, and will have given you the error message you got. The problem is that the error message you got was just a default one, and so that’s why it wasn’t helpful in pinpointing the error. Your error most probably wasn’t anything to do with the function not being marked payable, or the value exceeding the current balance.

This is easily done, and catches most of us out the first time we make that mistake

Adding an error message to your require() statements, is always a good idea, because if the revert is caused by one of them, the message you added will appear as part of the error message you get in the Remix console. If one of your own error messages doesn’t appear, you know that the transaction reverted because of something else.

Hopefully that will solve the problem. If not, then post a copy of your code for both contracts and I’ll take a look and investigate further

[KennethJP]

Hey guys, is the assignments mentioned in the value calls video the Multi Sig project? or am i missing a video ?