solc update to 0.4.18 (use view and pure 🎉) #169
Conversation
|
Ready for review |
| @@ -22,7 +23,7 @@ contract DelegateProxy { | |||
| } | |||
| } | |||
|
|
|||
| function isContract(address _target) constant internal returns (bool) { | |||
| function isContract(address _target) internal view returns (bool) { | |||
There was a problem hiding this comment.
@izqui Do you know if there's a list of assembly opcodes that force a function to be view instead of pure somewhere? The solidity docs are ambiguous as to which ones might do, but I noticed this is marked view (due to extcodesize?) while the ones that deal purely with memory are marked pure.
There was a problem hiding this comment.
I think it is checked at compile-time @sohkai, so anything that calls SSTORE or SLOAD will error out. No runtime enforcement
There was a problem hiding this comment.
AFAIK they're not enforced at compile-time yet (see warnings in the docs).
I'm guessing any instruction that deals with on-chain storage will force view, so:
sstoresload- every instruction in the "State queries" category
There was a problem hiding this comment.
Pure is for functions that dont access state (storage nor memory outside function scope)
View for functions that only read state (sload)
Functions that can modify state dont have special modifiers.
| @@ -359,11 +361,11 @@ contract MiniMeToken is Controlled { | |||
| bool _transfersEnabled | |||
| ) public returns(address) | |||
| { | |||
| if (_snapshotBlock == 0) | |||
| _snapshotBlock = block.number; | |||
| uint256 snapshot = _snapshotBlock == 0 ? block.number - 1 : _snapshotBlock; | |||
There was a problem hiding this comment.
Was there a reason for changing this to be block.number - 1 rather than block.number when _snapshotBlock == 0?
There was a problem hiding this comment.
Yes, there is a small race condition that can happen if using the same block number. We don't use it anywhere but fixed it jic
| @@ -1,5 +1,7 @@ | |||
| pragma solidity ^0.4.6; | |||
There was a problem hiding this comment.
Should we update this to ^0.4.18 as well?
There was a problem hiding this comment.
Didn't bother to update anything ERC677 or MiniMe as those are getting replace soon.
| @@ -1,4 +1,4 @@ | |||
| pragma solidity 0.4.15; | |||
| pragma solidity 0.4.18; | |||
There was a problem hiding this comment.
Is there some special restriction on putting/avoiding the ^ flag?
There was a problem hiding this comment.
Yes. All files that are used as interfaces have the ^, files that will be deployed have a compiler version fixed for security reasons. In the case of aragon apps all have to be fixed
There was a problem hiding this comment.
Closes #167
Also locks critical contracts to 0.4.18 and 'interfaces' to
^0.4.18, so it closes #158 and closes #160 (cc @sohkai please review this)Didn't bother to update anything ERC677 or MiniMe as those are getting replace soon.