Inheritance Assignment

Ethereum Smart Contract Programming 101
#1615

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

Destroyable.sol

pragma solidity 0.7.5;

import "./Ownable.sol";

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

Bank.sol

pragma solidity 0.7.5;

import "./Destroyable.sol";

contract Bank is Ownable, Destroyable{

I left out a chunk of the main bank file because I figured it just needed the inheritance part but if you need more I can post it! I also had a queston about naming convention. Is it wise to name the function as I did? Is there a problem with having the function named so closely to the built-in selfdestruct()?

1 Like
#1616

Great solution @0xsundown :ok_hand:

No that’s fine :+1:

But you can also remove Ownable from the Bank contract header. Destroyable already inherits Ownable, and so even if Bank only inherits Destroyable explicitly, it will still inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure…

contract A { ... }
contract B is A { ... }
contract C is B { ... }

I don’t know if it was unintentional, but your code already demonstrates that Bank.sol doesn’t need to import Ownable.sol. If Bank needed to explicity inherit Ownable — and include it in the Bank contract header as you have — then it would also need to import Ownable.sol.

Great question!

As you’ve probably already discovered, the compiler throws an error if you try to name the “wrapping” function selfdestruct because this is already the name of the in-built selfdestruct method. You are right, and wise, to consider whether using an almost identical name is a good idea, because in some situations this could definitely cause confusion. In terms of the smart contract code executing successfuly and securely, the degree of difference between the two names is irrelevant. But we do need to consider whether such a close similarity could lead to confusion when reading or developing the contract.

I actually don’t think it’s a problem here, because when I read your code, it’s obvious which is the main “wrapping” function (the function called by the contract owner from the external service), and which is the in-built selfdestruct method provided by Solidity. As with the in-built transfer method, selfdestruct can only be implemented within a function body, and so this further makes the distinction clear.

As the whole purpose of the “wrapping” function is essentially to call selfdestruct and destroy the contract, then I actually think the fact that you’ve chosen very similar names highlights the connection between them in a positive and helpful way :+1:

Just let me know if you have any further questions :slight_smile:

2 Likes
#1617

bank.sol

//SPDX-License-Identifier: UNLICENSED
pragma solidity 0.7.5;

import "./destroyable.sol";

contract Bank is Destroyable {
    
    mapping(address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    function deposit(uint qty) public payable returns (uint)  {
        balance[msg.sender] += qty; //msg.value;
        //emit depositDone(msg.value, msg.sender);
        emit depositDone(qty, 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, "Balance not sufficient");
        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;
    }
    
}

destroyable.sol

//SPDX-License-Identifier: UNLICENSED
pragma solidity 0.7.5;

import "./ownable.sol";

contract Destroyable is Ownable {

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

ownable.sol

//SPDX-License-Identifier: UNLICENSED
pragma solidity 0.7.5;

contract Ownable {
    address payable internal owner;

    modifier onlyOwner {
            require(msg.sender == owner, "This action is only available to the contract owner");
            _; //run the function
        }

    constructor() { 
        owner = msg.sender;
    }
}
1 Like
#1618

In the deposit function in bank.sol proposed as solution (link to Github file), I don’t totally understand how msg.value works and how to fill it up in Remix when running a transaction. I actually had to change msg.value to a parameter uint qty to be able to work:

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

1 Like
#1619

I have a small question. I first wrote my own code, which you can see below. I then checked the solution. It seems to work exactly the same, but could someone give me a second opinion?

The Bank contract

pragma solidity 0.7.5;

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

contract Bank is Ownable, Destroyable {

The Ownable contract

pragma solidity 0.7.5; 

contract Ownable {
    address owner;

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

    constructor() {
        owner = msg.sender;
    }
}

The Destroyable contract

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable{

    function destroy() public onlyOwner() {
        selfdestruct(msg.sender);
    }
}
1 Like
#1620

Hey Cesc,

That photo-gif makes me smile — is it a selfie? :grin:

Before I answer your question …

Your inheritance structure is well coded, and your solution will meet all of the assignment objectives, as long as when you call your deposit() function you input the same uint value in units of Wei (for the qty argument, next to the red deposit function-call button) as the Ether value you enter in the Value field (just below the Gas Limit in the same Deploy & Run Transactions panel in Remix) e.g.

Value field & adjacent dropdown:
  5 Ether   or   5000000000 Gwei (9 x 0’s)   or   5000000000000000000 Wei (18 x 0’s)

uint qty argument:
  5000000000000000000 (18 x 0’s)

That way, the increase in the user’s individual balance in the mapping (recording their share of the total contract balance) will be the same as the actual amount of Ether (in units of Wei) added to the contract balance.

But this is a very long-winded, and unnecessary, way of doing this (see my answer to your question below).

If you only call the deposit() function with the uint qty argument (without an Ether value), then the mapping will record an amount of Ether which that user is entitled to withdraw from the contract (their share of the total contract balance), but without actually transferring any Ether from the user’s external address balance (shown in the Account field, near the top of the Deploy & Run Transactions panel) to the contract address balance. In other words, the mapping will record individual user balances (amounts each user is entitled to withdraw from the contract) but the contract itself won’t actually hold any Ether for these users to withdraw.

The first thing to understand is that we only mark a function as payable when it needs to receive Ether from an external address to add to the contract address balance e.g. the deposit() function. In Remix, the Value field (and adjacent dropdown) makes it easy to send Ether from an external address to a deployed contract. Whatever value we enter here will be sent to the payable function we call with a red button, and automatically added to the contract address balance. The contract address is the address shown next to the name of the deployed contract below where it says Deployed Contracts at the bottom of the Deploy & Run Transactions panel. When we mark a function as payable, Remix will highlight this by giving us a red button to call that function (highlighting that the function expects to receive an Ether value which has been input in the Value field).

In our function body, in the same way that msg.sender will always reference the address that calls the function (the address showing in the Account field when the function-call button is clicked), msg.value will always reference the Ether value (in the Value field and adjacent dropdown) sent to the function from the caller’s (msg.sender's) external address balance.

In the same way that argument values input into a function can be referenced within the function body using the names of their parameters (defined between the parentheses after the function name in the function header), both the calling address and any Ether value sent to a payable function can also be referenced, but without the need to explicity define them as parameters. They are like in-built parameters (pre-defined in Solidity’s syntax) which we can reference using msg.sender and msg.value.

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 this same Ether value (always in units of Wei) 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. By using msg.value, instead of a separate uint qty parameter, we ensure that whatever units the Ether value has been expressed in (Ether, Gwei etc.) in the Value field, the same uint equivalent in Wei will always be added to the individual user’s balance in the mapping.

So that’s why the deposit() function doesn’t need any explicit parameters, and doesn’t need to be called with any arguments, because the only values it needs to reference in the code in its function body are the caller’s address (msg.sender) and the Ether value sent to it (msg.value).

Calling getBalance() will retrieve the caller’s individual share (their entitlement) of the total amount of Ether held in the contract. If you also add the following function to your contract, you can call it to retrieve the current total contract address balance (total amount of Ether held in the contract).

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

Any address can call this function and it will always return the total contract balance (not the caller’s individual share of that total balance). In our Bank contract, before we add the external value call to the Government contract (in the last course video before the final Multisig project), the amount returned by getContractBalance() should always be equal to the sum of all the values returned by getBalance() for each individual user with funds in the contract.

We don’t (and shouldn’t) 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. Effectively, the withdraw() function does the opposite of what the deposit() function does: it deducts Ether from the contract balance and transfers it to the caller’s external address balance. It does this using the in-built address member transfer (which is like a method called on a payable address)…

<address payable>.transfer(uint amount);

Notice, however, that, unlike the deposit() function, the withdraw() function does need to include an explicit uint parameter for the withdrawal amount, because, unlike with payable functions, there is no in-built msg.value equivalent available for us to use. In any case, the set syntax of the transfer method requires it to be called with a uint value. This uint value needs to come from somewhere, and in the withdraw() function it comes from the requested withdrawal amount input by the caller.

The separate transfer() function (not the special transfer method in the withdraw function) shouldn’t be marked payable either. 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 contract balance) i.e. the recipient’s entitlement (balance) is increased by the same amount the sender’s entitlement is reduced.

I hope that’s helped to fill in some missing links in your understanding. You may also 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, in Remix, if it is working correctly.

There’s probably a lot of information here to absorb, so take your time to think it all through and do some of your own testing, and when you’re ready, just let me know if anything is still unclear, or if you have any further questions :slight_smile:

#1621

Hi @simfin94,

Your solution is great, and it will work exactly like the model solution :ok_hand:

In fact, the 2 things you’ve done differently are actually improvements :muscle:

(1)  In your destroy() function …

… calling selfdestruct() with msg.sender directly (as you have done) is more concise than adding the additional step with the local receiver variable …

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

We are using Solidity v0.7.5 in this course, and prior to v0.8 msg.sender is already a payable address by default, and so doesn’t need to be explicitly converted whenever the syntax requires it to reference a payable address e.g. as the payable address argument in the selfdestruct() function call, or as the payable address the transfer method is called on. However, from Solidity v.0.8 msg.sender is non-payable by default, which would mean having to explicity convert msg.sender to a payable address when necessary. But even so, we can still do this directly within the function call itself, without having to include an additional local payable variable e.g.

// Solidity v0.8 syntax
selfdestruct(payable(msg.sender));

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

(2) Unlike functions, the visibility of state variables defaults to internal when the visibility keyword is omitted; unlike public and private , the internal keyword is optional. So, your state variable declaration in Ownable …

… is a more concise alternative to …

address internal owner;

Both work, but including the internal keyword here is unnecessary. It was probably included in the model solution in order to highlight the fact that owner has internal visibility, and so will be available in the derived contract. In practice, we would probably only want to explicity include the internal keyword in a state variable definition for emphasis or clarity; for example, if omitting it risks anyone reading or working with the code thinking otherwise.

Just one final observation …

Destroyable already inherits Ownable, and so Bank only needs to explicity inherit Destroyable, because it will still inherit Ownable implicitly via Destroyable. This will give you a much “cleaner” multi-level inheritance structure…

contract A { ... }
contract B is A { ... }
contract C is B { ... }

Just let me know if you have any further questions :slight_smile:

#1622

Thanks a lot @jon_m! It’s a super-helpful explanation :raised_hands: . It MAKES a lot of sense now that the Solidity standard defines he msg.value, msg.sender and the payable modifier. Here’s a little diagram I made:

msg_sender_msg_value
msg_sender_msg_value1475×1052 159 KB

One small thing I still don’t fully grasp though is how does Solidity do it to avoid the double-spending problem with the payable functions:

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

It is clear that after running deposit() the balance of msg.sender is increased with an amount of ETH (or whatever other unit that is chosen) as defined in msg.value. Furthermore, when I look in Remix, the msg.sender account is, in fact, credited (subtracted) that amount. But this seems to be happening automagically. It is as though when using payable and increasing the total contract balance, Solidity knows it has to credit the amount from msg.sender wallet.

This is in stark contrast with how it happens when we use withdraw(uint amount). As you correctly point out, this function does not have the payable modifier (because the account does not receive any money), but notice also how it doesn’t have something that for the lack of a better word could be called the creditable modifier. It is up to the programmer to make sure both operations (debiting the receiver and crediting the sender) are in fact happening:

        balance[msg.sender] -= amount;
        msg.sender.transfer(amount);

It would make more sense in my mind if the deposit() function forced you to do something like msg.sender.credit(amount); (which does not exist of course :slight_smile:) . There has to be some logic somehow to make sure that ETHs are not (1) created out of the blue; and the opposite (2) burned without the programmer explicitly saying so. But I can’t wrap my head around how this works other than just accepting this is how payable works and following blindly this syntax as a fact of life.

1 Like
#1623

Hi Cesc,

Nice diagram :ok_hand:

The payable function in the smart contract doesn’t deduct the ETH value from the sender’s (the caller’s) external address balance. This is why it doesn’t need an equivalent to the transfer method …

The calling address (i.e. msg.sender) chooses to call the deposit() function and send an ETH value to it (which the function can receive because it’s marked payable). Each Ethereum address on the Ethereum network has an account balance which records the amount of ETH held by that address in units of Wei. When an external address calls a specific function which sends a transaction to the network, the EVM will ensure the gas cost for that transaction is deducted from the calling address’s account balance. If the address also makes a value call, sending an ETH value to a payable function, then the EVM will ensure this value in uints of Wei is also deducted from the calling address’s account balance. This is what you see reflected in the Account field in Remix. The EVM works with units of Wei. The Remix user-interface has converted the external address account balances from Wei into Ether.

The Solidity syntax provides the smart contract developer with msg.sender and msg.value as a convenient way to program what the smart contract does (if anything) with the calling address value, and/or the Wei value sent to the smart contract. The Solidity keyword payable allows the developer to control how the smart contract can receive Ether (i.e. via which functions). When a smart contract receives Ether via one of its payable functions, the EVM will ensure that the equivalent uints of Wei are added to the contract address’s account balance.

In contrast, for Ether to be deducted from a smart contract’s address balance and sent to an external address’s account …

  1. The smart contract needs a function which can be called with the required information; and
  2. Within this function, the developer needs to be able to program when a specific amount of Wei is deducted from the contract’s address balance and sent to the external address of a specific recipient. The pre-defined syntax Solidity provides for this includes, amongst others, the transfer method:   <address payable>.transfer(uint amount)

Whenever Ether is successfully received by, or sent from, a smart contract, the EVM will ensure that the amount in uints of Wei is either added to, or deducted from, the smart contract address’s account balance. If there are multiple users of the smart contract, each entitled to a share of the total contract balance, then the contract also needs to record and track each user’s changing entitlement. This internal accounting function is performed in our Bank contract by the mapping. A user’s individual balance in the mapping is adjusted …

  • for each deposit with   balance[msg.sender] += msg.value;   which ensures the amount of Wei added to the contract address balance is reflected by an equivalent increase in the depositing address’s entitlement to (i.e. share of) the total amount of Ether held in the contract.

  • for each withdrawal with   balance[msg.sender] -= amount;   which ensures the amount of Wei deducted from the contract address balance is reflected by an equivalent decrease in the withdrawing address’s entitlement to (i.e. share of) the total amount of Ether held in the contract.

The first line of code, here, is not “crediting” the sender, because the sender of the Ether is the contract address. As I’ve explained above, this first line of code adjusts the individual user’s share of the total contract balance. Their individual share is mapped to their external address in the balance mapping. Their address, here, is the key to their individual share in the mapping, and acts as an identifier for that user. The second line of code is both “crediting” the contract balance and “debiting” the receiving external address balance with the same amount.

I’m not sure whether I’ve fully addressed or answered your following question …

… but hopefully, I’ve gone some way towards helping you understand some more of the logic behind the syntax :sweat_smile:

#1624 
// SPDX-License-Identifier: Leluk911
pragma solidity 0.8.7;
  
  contract Owner {
      // permanent owner
      //------> address public immutable owner;
      // mutable owner
      address payable public owner;

      constructor(){
          // set first or only owner
          owner = payable(msg.sender);
      }
      modifier OnlyOwner{
          // only owner access
          require(msg.sender==owner,"not Owner");
          _;
      }
    // useless with "immutable owner"
      function setOwner(address payable _new_owner)public OnlyOwner{
          // only owner set other owner
          owner = _new_owner;
      }
  }

// SPDX-License-Identifier: Leluk911
pragma solidity 0.8.7;
    // import Owner set who destroyed contract and liquidation ETH
   import "./Owener.sol";

  contract Self_destroyed is Owner{
      
      function destroyed()public OnlyOwner{
           selfdestruct(owner);
      }
  }

/ SPDX-License-Identifier: Leluk911
pragma solidity 0.8.7;

// import only Self_destroyed.sol because this child "./Owener.sol"
import "./Self_destroyed.sol";

contract Bank_Eth is Self_destroyed{
    // deposit receipt Eth
    mapping(address=>uint) public receipt;
    // variable modifier 
    bool paused=false; 

........eccc....
2 Likes
#1625

Destroyable.sol

pragma solidity 0.7.5;

import "./Ownable.sol";

contract Destroyable is Ownable {

    function destroy() public onlyOwner {

        selfdestruct(owner);

    }

}

Ownable.sol

pragma solidity 0.7.5;

contract Ownable {

    address payable public owner;

    modifier onlyOwner {

        require(msg.sender == owner);

        _; //run the function

    }

    constructor(){

        owner = msg.sender;

    }

}

helloworld.sol

pragma solidity 0.7.5;

import "./Destroyable.sol";

contract Bank is Destroyable {
1 Like
#1626

Ownable.sol

contract Ownable {

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

     constructor(){
        owner = msg.sender;
    }

}```

Destroyable.sol

import "./Ownable.sol";

contract Destroyable is Ownable {

    function destroy() public onlyOwner {

        selfdestruct(msg.sender);
    }
}

Bank.sol

pragma solidity 0.7.5;

import "./Destroyable.sol";

contract Bank is Destroyable {
1 Like
#1627

pragma solidity 0.7.5;

contract Ownable{

address public owner;

constructor(){

    owner=msg.sender;

}

modifier onlyOwner{

    require(msg.sender== owner);

    _;

}

}

contract Destroyable is Ownable{

function destroyContract() public payable{

 require ( owner == msg.sender, "You are not the owner");

 selfdestruct(msg.sender);

}

}

contract Bank is Destroyable{

mapping(address => uint)balance;

// This function will deposit money to the contract

function deposit() public payable returns(uint){

    balance[msg.sender]+=msg.value;

    return balance[msg.sender];

   

    }

//this function will withdraw the amount from the contract to the address calling this function    

function withdraw(uint amount)  public  {

    require(amount <= balance[msg.sender],"Balance is not sufficient");

    msg.sender.transfer(amount);

    balance[msg.sender]-=amount;

   

}

//this function will get the balance of address executing this function

function getyourBalance() public view returns(uint){

    return balance[msg.sender];

}

//this function will get you the total balance of ether in your contract

function CheckBalanceofContract() public view returns(uint){

    return address(this).balance;

}

}

1 Like
#1628

Bank contract

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

contract Bank is Ownable, Destroyable {
    
    mapping(address => uint) balance;
    
    event depositDone(uint amount, address indexed depositedTo);
    
    function deposit() public payable returns (uint)  {
        balance[msg.sender] = balance[msg.sender] + msg.value;
        emit depositDone(msg.value, msg.sender);
        return balance[msg.sender];
    }
    
    function withdraw(uint amount) public onlyOwner 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, "You do not have enough ETH");
        require(msg.sender != recipient, "Error");
        
        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;
    }
}

Ownable

pragma solidity 0.7.5;

contract Ownable {

    address 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 {

    selfdestruct(msg.sender);

  }

}
1 Like
#1629

Another excellent solution, @LELUK911 :muscle:

Your implementation of a multi-level inheritance structure is the best approach. Bank_Eth only needs to explicitly inherit Self_destroyed, because it will implicitly inherit Owner via Self_destroyed.

You have converted msg.sender to a payable address where the Solidity v0.8 syntax you are using requires this …

The setOwner() function you’ve added to the Owner contract is well coded and appropriate. You have correctly commented that the owner variable cannot be marked immutable if the owner address can be changed with the setOwner() function after deployment. Otherwise, you could definitely define owner as immutable, as your alternative code suggests, although it would also still need to be marked payable…

address payable public immutable owner;

… unless you convert owner to a payable address only when it needs to be, within the destroyed() function itself e.g.

function destroyed() public OnlyOwner {
   selfdestruct(payable(owner));
}

Let me know if you have any questions … and keep up the great coding! :slight_smile:

1 Like
#1630

An excellent solution @Techman120 :muscle:

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

#1631

Nice solution @JaimeAS :ok_hand:

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

Don’t forget to specify the required compiler version for each separate file, by including a pragma statement in each one. This may just be a copy-and-paste error when posting your code, but if it isn’t, the compiler should have generated orange warnings for omitting these.

It is not good practice to omit the pragma statement from any Solidity file, because we should always specify the Solidity compiler version (or range of versions) which our code is written to be compatible with.
Because the Bank contract inherits functionality from Destroyable and Ownable, even if the pragma statements are omitted from either or even both of their files, Remix will still attempt to compile Bank as a single contract together with the code it inherits. Bank will compile and deploy successfully as long as the code in both of the inherited contracts is compatible with the Solidity version declared in the pragma statement at the top of the file containing Bank. However, the compiler still generates orange warnings, highlighting that it’s best practice to include pragma statements in each file.
In the absence of a pragma statement, it seems that Remix will always try to find a suitable compiler version, although this doesn’t seem to be 100% reliable.
So, in conslusion, always include a pragma statement at the top of every .sol file.

Let me know if you have any questions :slight_smile:

#1632

Hi @imran82,

Your solution works and meets all of the assignment objectives :ok_hand: Your implementation of a multi-level inheritance structure is also the best approach: Bank only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable.

A few observations:

(1) Your destroyContract() function should not be marked payable, because it doesn’t need to receive 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 track of their share of the total funds held in the contract.
In contrast, the destroyContract() function triggers the selfdestruct method which, in addition to destroying the contract, also deducts the total amount of Ether from the contract balance and sends it to the payable address argument (msg.sender) which selfdestruct is called with, and which in our case can only ever be the contract owner’s address.

(2) You don’t need to include the require() statement in your destroyContract() function …

… because the inherited onlyOwner modifier is already available within Destroyable to add to the destroyContract() function header, and thereby prevent any address other than the contract owner from executing this function, destroying the contract, and receiving any Ether still remaining in the contract.

You could, however, add the error message "You are not the owner" to the require() statement in your onlyOwner modifier.

// Suggested improvements to address the points made in (1) and (2) above
function destroyContract() public onlyOwner {
    selfdestruct(msg.sender);
}

(3) By including all three contracts within the same file, your solution works with only one pragma statement, and without having to add any import statements. However, best practice is to place each contract in a separate file — no matter how small any of them are — each with its own pragma statement, and to import the parent contract file(s) where this is necessary for the successful implementation of the inheritance structure. Doing this makes your code more modular and, therefore, also more re-usable and easier to develop further. Improved modularity and code re-usability are both key reasons for choosing to implement inheritance.

Let me know if anything is unclear, or if you have any questions :slight_smile:

#1633

Hi @Arch.xyz,

Your code meets the assignment objectives. Your inheritance structure is well coded, but Bank only actually needs to explicitly inherit Destroyable, because it will inherit Ownable implicitly via Destroyable. This will give you a multi-level inheritance structure.

However, your withdraw() function is no longer adjusting the user’s individual balance in the mapping, because it no longer includes   balance[msg.sender] -= amount;
This is obviously a serious bug, and needs to be corrected.

You’ve also modified the withdraw() function with the onlyOwner modifier, but this means that only the contract owner will be able to withdraw funds while the Bank contract is deployed and operating normally. The contract allows multiple addresses to deposit funds (we are simulating a bank with bank account holders) and so, while the contract is still deployed, it only seems fair that all users should be able to withdraw their funds as well, don’t you think? :sweat_smile: Your withdraw() function header didn’t include the onlyOwner modifier before. I think during the course we were adding and removing onlyOwner from various functions to demonstrate different scenarios, and I think it might have ended up being left in the withdraw() function in the solution code for this assignment by mistake!

Let me know if you have any questions :slight_smile:

#1634

Destroyable.sol

pragma solidity 0.8.7;

import "./Ownable.sol";

contract Destroyable is Ownable {

    function destroy() public onlyOwner{

        selfdestruct(payable(msg.sender));

    }

}

bank contract.sol

pragma solidity 0.8.7;

import "./Ownable.sol";

import "./Destroyable.sol";

contract Bank is Ownable, Destroyable {
1 Like