Youâre nearly there @Gene03, but your derived contract wonât compile because you seem to have got your contract and file names mixed upâŚ
The name of your parent contract is DestroyableâŚ
And it looks like the name of the file is Destructible.sol, because thatâs what youâre importing âŚ
But to successfully inherit the parent contract you need to include its contract name in the derived contract header, not its file name âŚ
contract HelloWorld is Destroyable { ... }
and not
Are you not following the most recent Ethereum Smart Contract Programming 101 course, which uses Solidity v0.7.5? âŚ
The old course used v0.5.12 and included a HelloWorld contract. Unless you are just finishing off the old course because you had already started it over a year ago, you should be following the course based on the more up-to-date syntax (Solidity v0.7), and which develops a Bank contract throughout the course (also used in the assignments) instead of HelloWorld.
Let me know if anything is unclear, or if you have any questions
(By the way, to make sure we donât get confused about which version we are discussing, Iâve deleted your first post containing just your Destroyable contract, because your second post contains what I assume is your updated version. I hope you donât mind!)
Nice solution, which meets all of the assignment objects @antonfriedmann
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.
However, by coding your inheritance structure this way, Bank.sol only needs to import Destroyable.sol, so you can remove the line:â import "./Ownable.sol";
//public will make owner to be seeable.
address payable public owner;
modifier onlyowner{
require(msg.sender == owner);
_; // run the functions.
}
constructor(){
owner = msg.sender;
}
}
destroyable.sol
pragma solidity 0.7.5;
import â./ownerable.solâ;
contract Destroyable is Ownerable {
function close() onlyowner public{
selfdestruct(owner);
}
when i copy paste the code from the reading into remix, i get an error that âtype address is not implicitly convertible to expected type address payableâ at the line where âowner = msg.senderâ
my initial solution was just to make the address not payable; does the address need to be payable?
then i read some feedback around here and found out that the âownerâ variable isnât even necessary, so Iâd rather just use msg.sender directly unless thereâs some need to abstract it into a variableâŚ
back in a while with some attempted solutionsâŚ
edit to above: then i went back to the ownable contract itself (i.e. in a separate pane/tab); setting THAT âownerâ to payable, resolved the issue in the destroyable contract! theyâre looking at each otherâs contracts within the same workspace! i had no ideaâŚ
THEN i reloaded the remix page and all errors went away⌠(i noticed the error log telling me i had something in there that was no longer there, so i reloaded the remix page itself and voila! no more errors!
then i changed the local variable anyway.
later edit:
my solution
âownable_contract.solâ
pragma solidity 0.7.5 ;
contract ownable {
address payable public owner;
constructor() {
owner = msg.sender;
}
modifier onlyOwner() {
require(msg.sender==owner, "only the owner can do that!");
_;
}
}
âmake_destroyable.solâ
pragma solidity 0.7.5;
import "./ownable_contract.sol";
contract make_destroyable is ownable {
function close() onlyOwner public {
selfdestruct(owner);
}
}
in order to test it, i first compiled all three, deployed only the âbankâ contract, and checked the following (in this order):
1 a non-permissioned (in this case, a âgetâ) function from the owner contract
2 a function permissioned as onlyOwner (in this case, a ââsetâ function)
3 switching accounts, and repeat the above two steps (expecting the latter to fail)
4 the âcloseâ, i.e. âselfdestructâ, function from same account as step 3 (expecting it to fail, e.g. by checking that a non-permissioned function still works, still âgettingâ in this case)
5 switching back to first account (i.e. owner, i.e. msg.sender) and repeat first two steps to check that the contract wasnât destroyed for only this address (is that even possible? âŚthe way i code? maybeâŚha!)
6 from same account trying the âcloseâ function again, this time expecting it to work (e.g. in this case, by checking âgetâ then setâ then âgetâ again expecting â0â, then no issue, then â0â again, as described in reading)
remaining questions:
in the solutions, address is not payable, and IS private; i think i understand privacy to be a safer default than âpublicâ, but is there a meaningful effect here?
in the solutions, the order in which âpragmaâŚâ is listed with respect to âimportâŚâ seems not to matter; do i understand correctly? (similarly there is no âpragmaâŚâ in the âownable.solâ solution on github; is it unnecessary because the compiler will only be used on the most derived contract?)
Iâve followed through your testing, and apart from the first âgetterâ, I assume you are then calling the deposit() and getBalance() functions â and perhaps performing a transfer as well â in Bank. Is that correct? You need to ensure you deposit some ether in the contract in order to test that, when selfdestruct() is called, the ether remaining in the contract is transferred to the contract ownerâs external address.
I also find it helpful to add an additional getter to the Bank contract, to check the total contract balance at various stages (especially just before selfdestruct is called) e.g.
function getContractBalance() public view returns(uint) {
return address(this).balance;
}
If selfdestruct() is executed successfully, it removes the contract from the blockchain, and so it can no longer be interacted with by any address.
When you checked these functions after calling close() with the owner address, you should have noticed that, despite the contract having been destroyed, the function calls were still successfull and didnât throw errors! We can be sure that the contract has been destroyed, because the getters now return zero values. However, a clear disadvantage with using selfdestruct() is that, after the contract has been destroyed, a payable function can still be called, by mistake, with an ether value and, instead of reverting, the ether will be lost. Therefore, if a smart contract deployed on the mainnet is ever destroyed, all users need to be notified and informed that they must not send any more funds to that contract address.
After destroying the contract, if you call the deposit() function with an ether value, you will see that the ether is deducted from the callerâs external address. If you then call getBalance() or getContractBalance(), either of these will return zero.
The potential repercussions of selfdestruct need to be fully understood before choosing to implement it; but for the purposes of this introductory course, it provides a simple and straightforward way to practise implementing inheritance, which is our main objective in this assignment. More effective alternatives exist, such as pausable contracts. These involve more advanced techniques and are therefore out of the scope of this introductory course, but you can learn about pausable contracts and other smart-contract emergency and security features, such as proxy contracts, in the Smart Contract Security course.
Remaining Questions
I assume youâre referring to the owner state variable in OwnableâŚ
The solution provided is just one of several alternatives. It marks the owner variable with internal (not private) visibility. In fact, state variables are internal by default, so owner will still have internal visibility if the keyword is omitted. State variables and functions with internal visibility are inherited, but are only available within the contract and its derived contract(s); they cannot be accessed externally. The only effect you would see by changing the visibility of owner from public to internal is that the getter (to retrieve the ownerâs address) would no longer be automatically generated.
If a state variable or function is marked with private visibility, then it can only be accessed from within the same contract (excluding any derived contracts). We should always aim to restrict access to only what is necessary; this increases security and, as you say, is safer.
In the solution, the owner state variable isnât declared as a payable address type because, in Destroyable, selfdestruct() is effectively called with msg.sender (via an intermediary local variable receiver), instead of owner. Prior to Solidity v0.8 msg.sender is payable by default. We can use msg.sender to reference the contract ownerâs address, because this is the only address that it can ever represent in the close() function, as a result of the onlyOwner modifier restricting who can access it.
If we only need the contract ownerâs address to be payable for the purposes of executing selfdestruct(), another alternative solution is to leave the owner state variable as non-payable, and explicitly convert the address to payable locally, within the function where we actually need to use it as payable. We can even do this directly within the selfdestruct() function call itself, as follows:
function close() public onlyOwner {
selfdestruct(payable(owner));
}
Yes ⌠the contract will still compile and deploy successfully using either order. However, personally, I prefer to place any import statements directly beneath the pragma statement, as you have done.
This is a good questionâŚ
It is not good practice to omit the pragma statement from any Solidity file, because we should always define the Solidity compiler version (or range of versions) which our code is written to be compatible with.
Because the Bank contract inherits functionality from the Destroyable and Ownable contracts, even if the pragma statements are omitted from either or even both of these inherited contracts, 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 issues orange/yellow warnings, highlighting that itâs best practice to include pragma statements in each file.
Having experimented a bit, it seems that, in the absense of a pragma statement, Remix will always try to find a suitable compiler version, although this doesnât seem very reliable.
In conslusion: always include a pragma statement at the top of every .sol file.
Below is my solution for the inheritance assignment.
One Question:
In my solution the Bank contract inherits from the other two contracts. However, the Ownable contract is parent to the Destroyable contract, and the Destroyable contract is parent to the Bank contract. Thus the Bank contract inherits from the Ownable contract through the Destroyable contract. Is this statement correct?
If yes, should I eliminate the âis Ownableâ in the Bank contract because it is redundant or should I keep the âis Ownableâ in the code because it has no negative effect (e.g. performance) and it actually makes it much easier to detect the inheritance relationships for anyone who reads the code?
File bankOwnable.sol
pragma solidity 0.5.7;
import "./ownable.sol";
import "./bankOwnableDestruction.sol";
contract Bank is Ownable, Destroyable{
...
}
File Ownable.sol
pragma solidity 0.5.7;
contract Ownable{
address payable public owner; // "paybale" makes address eligible to receive funds; "public" makes state variable eligible for inheritance
modifier onlyOwner{
require(msg.sender == owner);
_;
}
constructor() public {
owner = msg.sender;
}
}
File bankOwnableDestruction.sol
pragma solidity 0.5.7;
import "./ownable.sol";
contract Destroyable is Ownable{
function close() public {
selfdestruct(owner);
}
}
i think thatâs correct; after the first getter function, i just used a quick and dirty âsetâ function to âtransferâ funds. good point about checking the remaining ether; i hadnât done so.
yes! saw that
payable takes arguments?! ⌠so many things i have yet to learnâŚgood thing i donât intend to actually use this for my bread and butter yet ha!
This isnât a function call, itâs just convenient syntax to convert a non-payable address, which is placed within the parentheses, to a payable one. In Solidity, parentheses can be used in this way to perform certain explicit type conversions e.g.
uint256 x = 50;
uint8 y = uint8(x);
string memory message = "Hello World!";
bytes memory z = bytes(message);
uint160 a = uint160(msg.sender);
address b = address(a);
bytes20 c = bytes20(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4);
address d = address(c);
Iâve just written this contract, which you can play around with. It uses the same explicit type conversions as my example in my last post. But this time you can output the converted values.
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.10;
contract TypeConversion {
uint8 y;
bytes z;
uint160 a;
address b;
bytes20 c;
address d;
function convertType() external {
uint256 x = 50;
y = uint8(x);
string memory message = "Hello World!";
z = bytes(message);
a = uint160(msg.sender);
b = address(a);
c = bytes20(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4);
d = address(c);
}
function getResults() external view returns(uint8, bytes memory, uint160, address, bytes20, address) {
return (y, z, a, b, c, d);
}
}
Your solution is almost there; but it wonât compile because youâre not importing the Destroyable.sol fileâŚ
âŚyouâre missing the .sol file extension.
Your implementation of a multi-level inheritance structure is the best approach, because it is more streamlined. HelloWorld only needs to explicitly inherit Destroyable, because it will implicitly inherit Ownable via Destroyable. However, this also means that you donât need to import Ownable.sol into HelloWorld.sol (just Destroyable.sol)âŚ
Iâve also noticed that you donât seem to have posted any of the other assignments in this course. Even if youâve watched and coded along with all of the videos, I would still encourage you to complete and submit the assignments, because these provide important practice and as such are an important part of the course. If you donât get enough practice, you may struggle with the 201 course which follows this one.
Also, donât forget to format your code before posting it. This makes it clearer, easier to read, and easier to spot any syntax errors. It also makes it easier to copy and paste if we need to deploy and test it. Follow the instructions in this FAQ:How to post code in the forum
Your solution is almost there⌠Everything compiles, your inheritance structure works, the contract can be destroyed by calling the close() function in Destroyable, and on destruction any ether still remaining in the contract will be transferred to the contract ownerâs external address You just have one problem to resolveâŚ
At the moment anyone can call close() and destroy the contract. The funds are safe, because only the contract owner can receive them. But the fact that any address can actually trigger selfdestruct is obviously not ideal
What functionality, inherited from Ownable, can you implement in Destroyable, to ensure that only the contract owner can call close() and trigger selfdestruct ?
Yes ⌠this is correctâŚ
Your implementation works, but there is no need to include the code for Bank to explicitly inherit Ownable. A multi-level inheritance structure is the best approach, and this is what youâve described in your statement. Bank only needs to explicitly inherit Destroyable because it will implicitly inherit Ownable via Destroyable.
Yes⌠thatâs correct â itâs redundant.
Your solution still works perfectly well, and you are right that it has no negative effect on performance.
I would actually say that using multi-level inheritance produces cleaner and clearer code, because it is more streamlined. By removing the redundant parent-child relationship, the true three-level, linear relationship is better reflected in the code.
As well as removing this, you should also remove the associated import statementâŚ
A couple of other observations âŚ
Whatâs happened to v0.7.5 ? Are you deliberately experimenting with the older v0.5, or is this just a slip? You probably noticed that the v0.5 syntax for the constructor was slightly different: it required visibility to be defined as either public or internal. From v0.7 visibility no longer needs to be defined for constructors.
You could also define this state variable with internal visibility and it would still be inherited. It would then still be available within the derived contract(s), but not externally, and the automatic getter would no longer be created. With state variables, internal visibility is the default, so the keyword doesnât need to be includedâŚ
// even though not stated, this state variable has internal visibility
address payable owner;
With functions, however, visibility must always be explicitly defined.
pragma solidity 0.7.5;
import "./ownable.sol";
contract Destroyable is Ownable{
function close() public onlyOwner {
selfdestruct(owner);
}
}
I have another question concerning the meaning of msg.sender:
The mistake with the onlyOwner modifier in my Destroyable contract is actually the result of a misconception about the meaning of msg.sender
From the tutorial of this academy and also from a written documentation (See âBlock and Transaction Propertiesâ), I assumed that msg.sender is equal to the current address that interacts with the contract, or in other words the address of the entity that calls the contract and its functions etc. I think this statement is correct, isnât it?
My misunderstanding was that I was mislead by the functionality of the Ownable conctract in the way that I assumed each time another address interacts with the contract, the constructor changes the value of the variable owner to the address of the party currently interacting with the contract. In the moment of writing the Ownable contract, I just missed the important fact that the constructor is executed only once when the contract is deployed. Since I do not change the variable owner somewhere else, it remains equal to the address of the party that deploys the contract.
To sum it up, are the following statements correct?
1.) msg.sender equals the address of the party/entity that currently interacts with the conctract (applies to both, the party that initially deployed the contract and all other parties that interact with the contract after its deployment)
2.) Using the Ownable contract, as shown above, is a best practice to store the address of the party that deployed the contract and can then be used for different purposes (in this case to selfdestruct the contract).
A final point:
I totally agree with the safety aspect related to selfdestruct - in case a smart contract has vulnerable code that is being exploited by an attacker, the designated contract owner can withdraw all remaining funds from the contract and mitigate the financial damage.
However, having one address with the power to withdraw all funds from a contract represents a strong concentration of power. I assume it is common practice that initiating the function to trigger selfdestruct requires the signature of multiple parties, but still the power remains in the hand of e.g. a few members of the developer team. My question actually is, is this a big/hot topic in the crypto space (the good old story about security vs. decentralization) and what is the general perception about this issue (feel free to post your own opinion if you want )
First, a âmistakeâ in the assignment I think : âThen, you should be able to deploy the Bank contract, perform some transactions and, finally, destroy the contract, transferring any remaining ether held in the contract to the contract owner.â
In fact, in our example, there is âowner = msg.senderâ in Ownable class. That means, any remaining ether in balance[msg.sender] will be always equals to balance of the owner cause this is the same address.
So, my code below donât work, because the transfer method try to transfer remaining ETH from and to the same balance. I think we have to rewrite lots of logic in order to perform a scenario where the msg.sender and owner will be different in order to check if the transfer of remainin ETH will work. I will see in the assignement solution what it will.
The good side of this âmistakeâ, if it is, I know use the debugger with Remix.
DESTROYABLE.SOL
pragma solidity 0.7.5;
import "./Ownable.sol";
contract Destroyable is Ownable
{
function close() public onlyOwner
{
selfdestruct(owner);
}
}