4

Two months ago, I wrote a smart contract with Remix.ethereum.org. Line 1 of my code has:

pragma solidity ^0.4.11;

It compiled fine with no errors and nor warnings. I tested every function in Ropsten and it worked fine.

Now, I'm getting closer to deploying it on the Main net. I see that Remix has changed. Now, it says:

Static Analysis raised 50 warning(s) that requires your attention.

Under the Analysis tab, there are many warnings, that I cannot figure out how to resolve. One example is,

Potential Violation of Checks-Effects-Interaction pattern in TestContract.testFunction(address): Could potentially lead to re-entrancy vulnerability. Note: Modifiers are currently not considered by this static analysis.

Another example is,

Gas requirement of function browser/TestContract 8.sol:MintableToken.transferFrom(address,address,uint256) unknown or not constant. If the gas requirement of a function is higher than the block gas limit, it cannot be executed. Please avoid loops in your functions or actions that modify large areas of storage (this includes clearing or copying arrays in storage)

Another example is,

Fallback function of contract browser/TestContract 8.sol:Test requires too much gas (null). If the fallback function requires more than 2300 gas, the contract cannot receive Ether.

Another example is,

Test.transferFrom(address,address,uint256): Potentially should be constant but is not. Note: Modifiers are currently not considered by this static analysis.

There are multiple occurrences of each of the above examples. Can I ignore all of these warnings? If not, how do I resolve them?

Under Settings, it was using Solidity version 0.4.11. If I change this to 0.4.17, I get dozens of warnings under the Compile tab as well. One example is,

browser/TestContract 8.sol:15:3 Warning: No visibility specified. Defaulting to "public":

function register(string key) {

^

Spanning multiple lines.

I can add "public" after the close bracket for the parameters in each of the functions to resolve the above warnings, but I'm not sure if that is the right thing to do.

Another warning example is,

browser/TestContract 8.sol:100:3 Warning: Function state mutability can be restricted to pure

function mil(uint256 a, uint256 b) internal constant returns (uint256) {

^

Spanning multiple lines.

I have no idea how to resolve the above warning.

Am I correct to assume that I can ignore 0.4.17 and go back to 0.4.11?

0.4.11 up to 0.4.16 will not show these warnings under the Compiler tab. Should I use 0.4.16?

My code can be seen here

3

In general

you can let a code run ignoring the compiler warnings. But it's always good not to do so. Most the time compiler warnings are warning you about something that may cause a bug in your code. Compared to compiler errors, the difference is your code is syntactically correct, hence can be compiled, but can lead to problems, specially when someone try to something which is not something you expect other's to do.

Compiler warnings indicate things that might cause problems or might have unintended effects that the programmer wasn't aware of.

A good read regarding this can be found here, and you may refer this question as well. And most importantly read the security considerations from solidity docs. That may save your life!

Anyhow compiler is not intelligent as you and it claims what it's lacking from in statements with Note: as follow,

Note: Modifiers are currently not considered by this static analysis.

Sometimes these notes may tell the occasions that you can safely ignore these warnings.


Example

One error warning you have mentioned is,

browser/TestContract 8.sol:15:3 Warning: No visibility specified. Defaulting to "public":

Let's say you have function A(uint val) {} you want it to be used only within the contract due to some prior validations you need to do (This is just an example ;)). And if the function is called without validation that may cause some unwanted executions. And the only use of function A is to use inside some other functions within the contract.

pragma solidity ^0.4.0;

contract Example {


    function A(uint val) {
       //do something with val 
    }

    function B(uint val){
        //validation of val
        A(val);
    }

}

Now if you keep the contract like above, since function A() is also by default set to public, anyone who use your contract can use function A with any uint value and that may cause the trouble. But if you had the access modifier of the function A declared as private this trouble won't cause since function A can't be accessed publicly. And as your requirement is also to use function A only inside the other functions within the contract your goal is also achieved.

As when no access modifier is declared, compiler assumes a function to be public and it warns you that it's setting the function to be public and ask you to check whether it should really be public. In the above case, you will get warning regarding both functions and setting function A to be private and function B to be public will remove the warnings because now the compiler know you are intentionally making function B public and function A private.

pragma solidity ^0.4.0;

contract Example {


    function A(uint val) private {
       //do something with val 
    }

    function B(uint val)public {
        //validation of val
        A(val);
    }

}

The warning regarding function B is being set to public is a type of warning that you can safely ignore as your intention also the same, but the warning regarding function A is something you should really consider.


What I suggest,

  • not to ignore the warning, but go through them and check whether they do really matter, with how you expect the code to behave and try to resolve them.

  • try to use the latest compiler version (atm 0.4.17), as newer stable versions tend to have less bugs and more features (hence it generate new warnings).


Some insight regarding some other warnings you get

browser/TestContract 8.sol:100:3 Warning: Function state mutability can be restricted to pure

You may refer this answer, this question regarding introduction of view and pure type functions.

Potential Violation of Checks-Effects-Interaction pattern in TestContract.testFunction(address):

This is for not following the pattern of Checks-Effects-Interactions as described here in docs. Refer the function that get this warning and follow the pattern defined in the docs to avoid warning.

and as per the code found in the link you posted, I think the closeSale() warning should probably be due to compiler assumes there can be a re-entrancy chance as you are calling the member event LogSaleClosed(uint256 issuedSupply, uint256 restrictedTokens) after interactions with token contract.

What I can suggest is having that event inside the contract token.You can see here for re-entrancy vulnerability. Its just saying if the bad code there let another contract to withdraw money several times because they change the state variable shares after sending transaction which might take some time leaving the other contract to call the same function several times because it keeps msg.sender value non-zero until the transaction get executed. If you see the good code there they first set shares[msg.sender] to zero first. Since you are the one who developed this, you know the flow of expected events and can be sure that re-entrancy won't occur it's safe to ignore it.

Test.transferFrom(address,address,uint256): Potentially should be constant but is not. Note: Modifiers are currently not considered by this static analysis.

What constant means that it would not be changing the state. That's no state variable is changed. So normally those functions are called constant functions and are defined with the keyword of constant . you may read this.

So if your functions are not changing any state variable you can declare them constant, solidity is reminding you about this.

  • I have “contract Contract1 is Contract2”. If Contract1 functions call Contract2 functions, but not called from other contracts or transactions, should Contract2 functions be “internal” or “private”? What about the other warnings such as “Potential Violation of Checks-Effects-Interaction…” or “Gas requirement of function…”? How do I resolve those? With this warning: “[function]: Potentially should be constant but is not”, the function is to transfer tokens. I do not see how this can be a constant. How do I resolve this warning? FYI, I added a link to my code on my original post. – Curt Oct 16 '17 at 14:09
  • Thanks for your reply. Remix used to provide the ABI /JSON interface code. Now, I don't see it. How do we get this? – Curt Oct 16 '17 at 14:14
  • yes, it has to be internal check here. – Achala Dissanayake Oct 16 '17 at 21:15
  • I updated the answer regarding checks-effects-interaction at the end of the answer – Achala Dissanayake Oct 16 '17 at 21:43
  • Thanks for your help! I'm getting closer. I resolved the Checks-Effects-Interaction warning for 2 of the 3 functions. I cannot for the TestSale.closeSale function. Please see the code at gist.github.com/anonymous/3497e6b4e09953a009a4fdbacd11946a I don't think this is resolvable. What do you think? I'm also getting "Potentially should be constant but is not" warning on 3 functions: TestToken.transfer, TestToken.transferFrom and TestSale.selfDestruct. Do you know how to resolve? – Curt Oct 16 '17 at 22:21
1

browser/TestContract 8.sol:100:3 Warning: Function state mutability can be restricted to pure

Answer: Functions can be declared pure in which case they promise not to read from or modify the state.

. http://solidity.readthedocs.io/en/develop/contracts.html#pure-functions

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.