Inheritance Assignment

[david.maluenda]

Adding a forgotten assignment, It has a little Extra on it, meaning it has a little more code to it then the lecture requires.

Bank

pragma solidity 0.7.5;

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

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

contract Bank is Ownable, Destroyable {

GovernmentInterface governmentInstance = GovernmentInterface(0xaE036c65C649172b43ef7156b009c6221B596B8b);


mapping(address => uint)balance;


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


function deposit() public payable onlyOwner 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);
    require(msg.sender != recipient);
    
    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);
}

}

Ownable
pragma solidity 0.7.5;

contract Ownable {

address internal owner;



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


constructor() {
    owner = msg.sender;
}

}

Destroyable

pragma solidity 0.7.5;

import “./Ownable.sol”;

contract Destroyable is Ownable {

function Destroy() public onlyOwner {
    address payable recipient = msg.sender;
    selfdestruct(recipient);
} 

}

[EduardoDaSilva]

Good catch!
Thank you

[jon_m]

Hi @david.maluenda,

Overall, your coding of the inheritance structure and the selfdestruct functionality is good

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.

@david.maluenda

address payable recipient = msg.sender;
selfdestruct(recipient);

Here, a concise alternative is to call selfdestruct() with msg.sender directly, instead of using the additional recipient variable.

selfdestruct(msg.sender);

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

---

Access to your deposit() function is now restricted by the onlyOwner modifier, which wasn’t included in the function header before. This should be removed, otherwise no one other than the contract owner will be able to deposit ether into the Bank contract — unless this is what you intend, and then for the owner to distribute this ether to other users via the transfer() function … ?

Don’t forget to address both of these additional points I mentioned about your Bank contract. It’s especially important to include the additional statement in your withdraw function to adjust the user’s individual balance, otherwise your contract will fail to prevent users from withdrawing more ether than they are entitled to.

Just let me know if you have any questions.

[david.maluenda]

Hi Jon,

Could you please show me in the code where and how you would do the changes?
It would give a more visual understanding.

Much appreciated and thanks for your input.

[Shadowstep2003]

How is the below? Contract bank code wasn’t really changed, just the headers to import

pragma solidity 0.7.5;

contract Ownable {
    
    address payable public owner; 
     
     // When placed in a function this will run first
    modifier onlyOwner {
     require(msg.sender == owner);
     _; // run the function that called this
    }
    
    constructor(){
     owner = msg.sender;
    }
     
}

pragma solidity 0.7.5;

import "./artifacts/Ownable.sol";

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

pragma solidity 0.7.5;

import "./Destroyable.sol";

contract Bank is Destroyable {

[jon_m]

Hi @david.maluenda,

Below each of the changes I suggested, I’ve copied the relevant part of your code and shown you how to modify it. I’ve commented out code which would need to be removed, and written comments to show where I’ve added code …

@jon_m

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.

pragma solidity 0.7.5;

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

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

contract Bank is /*Ownable, */Destroyable { ... }

---

@jon_m

a concise alternative is to call selfdestruct() with msg.sender directly, instead of using the additional recipient variable.

function Destroy() public onlyOwner {
    // address payable recipient = msg.sender;
    selfdestruct(/*recipient*/msg.sender); // recipient REPLACED with msg.sender
}

---

@jon_m

Access to your deposit() function is now restricted by the onlyOwner modifier, which wasn’t included in the function header before. This should be removed, otherwise no one other than the contract owner will be able to deposit ether into the Bank contract — unless this is what you intend, and then for the owner to distribute this ether to other users via the transfer() function … ?

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

---

@jon_m

It’s especially important to include the additional statement in your withdraw function to adjust the user’s individual balance, otherwise your contract will fail to prevent users from withdrawing more ether than they are entitled to.

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

In order to reduce security risk, the statements should be in the following order within the function body:

1. Check inputs (require statements)
2. Effects (update the contract state)
3. External interactions

It’s important to modify the contract state for the reduction in the individual user’s balance…

balance[msg.sender] -= amount;

before actually transferring the funds out of the contract to the external address…

msg.sender.transfer(amount);

… just in case there is an attack after the transfer, but before the state is modified to reflect this operation. You’ll learn about the type of attack this prevents, and how it does it, in the courses which follow this one.

---

@jon_m

• 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.

function transfer(address recipient, uint amount) public {
    require(balance[msg.sender] >= amount);
    require(msg.sender != recipient);
    
    uint balanceBeforeSending = balance[msg.sender];
    _transfer(msg.sender, recipient, amount);
    
    governmentInstance.addTransaction(msg.sender, recipient, amount);
    
    assert(balance[msg.sender] == balanceBeforeSending - amount);
    emit transferDone(msg.sender, recipient, amount);  // MOVED TO HERE
    // Note that you need to change two of the arguments
}

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

I hope that’s clearer, but just let me know if you have any further questions

[jon_m]

Another great solution @Shadowstep2003

Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.

Keep up the great coding!

[alp257]

// Ownable.sol file
contract Ownable {

address payable owner;

constructor() {
    owner = msg.sender;
}

modifier onlyOwner {
    require(msg.sender == owner, "You're not the owner of the contract.");
    _; // run the function
}

}

// Destroyable.sol file

import “./Ownable.sol”;

contract Destroyable is Ownable {

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

}

// Bank.sol file

pragma solidity 0.7.5;

import “./Destroyable.sol”;

contract Bank is Destroyable {
…
}

[CryptoByNight]

pragma solidity 0.7.5;

contract Ownable {
    
    address internal owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;
    }
    
    constructor() {
        owner = msg.sender;
    }
    
}


pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {
    
    function destroy() public onlyOwner {
        selfdestruct(msg.sender);
    }
    
}


pragma solidity 0.7.5;

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

contract Bank is Ownable, Destroyable {
    
    mapping(address => uint) balance;

[jon_m]

Great solution @alp257

Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.

Keep up the great coding!

[jon_m]

Nice solution @CryptoByNight

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.

Just let me know if you have any questions.

[Jules]

[Jules]

do I need to import both ownable and destroyable into my Bank contract?

[CryptoByNight]

Yes good call. That makes sense.

[jon_m]

Nice solution @Jules

@Jules

do I need to import both ownable and destroyable into my Bank contract?

No you don’t need to. Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.

It will still work in exactly the same way if Bank explicitly inherits both Destroyable and Ownable, and if both destroyable.sol and ownable.sol are imported, but it’s unnecessary.

[HRMS2021]

Bank.sol

pragma solidity 0.7.5;
import "./Ownable.sol";
import "./Destroyable.sol";

contract Bank is Ownable {
    

Destroyable.sol

mport "./Ownable.sol";
pragma solidity >0.7.5;

contract Destroyable is Ownable {
 
 function destroy() public onlyOwner { 
  address payabble receiver = msg.sender;
  selfdestruct(receiver);
 }
}

Ownable.sol

pragma solidity 0.7.5;

contract Ownable {
    address public owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _;//run the function
    }
    
      constructor(){
        owner = msg.sender;
    }
}

[jon_m]

Hi @HRMS2021,

There are a few issues here, but they are easily fixed:

(1) The following two lines of code in Destroyable.sol contain misspelt keywords. These generate red compiler errors, but maybe these are just copy-and-paste errors and aren’t in your code in Remix …

@HRMS2021

mport "./Ownable.sol";

@HRMS2021

address payabble receiver = msg.sender;

(2) Bank can’t be deployed because of the following compiling issue caused by Destroyable.sol …

Imported files must have a compiler declaration that is compatible i.e. both files must be able to be compiled using the same version of Solidity.

Your Ownable.sol can only be compiled using v0.7.5. It cannot be imported into your Destroyable.sol because your Destroyable.sol requires a compiler version of v0.7.6 or above …

@HRMS2021

pragma solidity >0.7.5;

Your Destroyable.sol also cannot be imported into your Bank.sol for the opposite reason: your Bank.sol can only be compiled using v0.7.5.

Remix should have generated red compiler errors for this. These errors won’t have highlighted a specific line of code, and so the error messages can’t be viewed by hovering over the red indicator highlighting the line number. However, they will be displayed in the Solidity Compiler panel — the Solidity Compiler icon indicates this with the number of error messages highlighted in red.

(3) Once the above issues are fixed, and your code compiles successfuly, you’ll be able to deploy your Bank contract. However, you’ll notice that the destroy() function is not available for the owner to call. Your selfdestruct functionality is coded correctly, but it’s not inherited by Bank. Can you correct this? When you have, you will be able to deploy Bank, perform some transactions, and then get the contract owner to destroy the Bank contract (by calling the destroy function from Remix). Destroying the contract will also transfer any remaining ether balance to the owner’s external address.

---

@HRMS2021

address payabble receiver = msg.sender;
selfdestruct(receiver);

Here, a concise alternative is to call selfdestruct() with msg.sender directly, instead of using the additional receiver variable.

selfdestruct(msg.sender);

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

Let me know if anything is unclear, or if you have any questions about how to correct your code for these issues

[HRMS2021]

Thank you @jon_m for your feedback, I will make changes as per your suggestions.

[Ruben99]

We define the Ownable class

pragma solidity 0.7.5;

contract Ownable {
    
    address internal owner;
    
    modifier onlyOwner {
        require(msg.sender == owner);
        _; //run the function
    }
    
    constructor(){
        owner = msg.sender;
    }
}

In the Destroyable contract we inherit from Ownable so only the owner can destroy the contract.

import "./ownable.sol";

pragma solidity 0.7.5;

contract Destroyable is Ownable{
    
    function destroy() public onlyOwner{
     selfdestruct(payable(msg.sender));
    }
}

Then in the main contract we inherit from destroyable.

import "./destroyable.sol";

pragma solidity 0.7.5;

contract Bank is Destroyable{

[jon_m]

Great solution @Ruben99

Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.

Just one comment…

@Ruben99

selfdestruct(payable(msg.sender));

This works, but you are using Solidity v0.7.5, and prior to Solidity v0.8 msg.sender is already a payable address by default. This means that whenever you want to use msg.sender as a payable address (such as here, as the argument selfdestruct is called with), you don’t have to explicitly convert it e.g.

selfdestruct(msg.sender);

However, from v.0.8 msg.sender is non-payable by default, and so whenever you are using v.0.8 and need to use msg.sender as a payable address, this requires the explicit conversion that you have used.

Let me know if you have any questions.

By the way, you seem to have missed out the Transfer Assignment — is that on purpose because it was too easy?

Keep up the great coding!