aragon / aragon-apps

(Aragon 1) Aragon apps developed by Aragon Core Devs (smart contracts + webapps)
https://aragon.org/discover
GNU Affero General Public License v3.0
388 stars 212 forks source link

Upgrade truffle and solc and use 'view' and 'pure' #60

Closed izqui closed 6 years ago

izqui commented 6 years ago

Similar to https://github.com/aragon/aragon-core/pull/169

In the case of the Finance app there can be some constant functions that need refactoring to convert to view, given that they modify state.

status-open-bounty commented 6 years ago

Balance: 0.000000 ETH Tokens: ANT: 10.00 Contract address: 0x4da8d3de333840e6580f3d86f5a2a4c382ef60fb Network: Mainnet Paid to: renexdev Visit https://openbounty.status.im to learn more.

renexdev commented 6 years ago

Coding & learning to solve this!

izqui commented 6 years ago

@renexdev sweet aragon/aragon-core#169 could be an inspiration :)

renexdev commented 6 years ago

I'm following that! Thanks

renexdev commented 6 years ago

@izqui, sorry by the newbie q, what's the purpose of the few sentences in onApprove fn (TokenManager.sol)... I feel I'm loosing something that's happening in the backend with TokenController...
function onApprove(address _owner, address _spender, uint _amount) public view returns (bool) { _owner; _spender; _amount;

izqui commented 6 years ago

That function doesn't do anything. The parameters are in the function body just so the linter doesn't protest about unused variables.

renexdev commented 6 years ago

Almost ready... re running install & tests + updating gist with obtained outputs... checking the warnings... In addition to js test, Is there any way to fine grained follow EVM commands & play with the defined apps Fn (like Remix) for such a big project?... I imagine that one way of doing that is using Remix with small pieces of the contract with its dependencies, but for sure this is not practical for agile dev... Tacking details of the pull request... https://gist.github.com/renexdev/a6325c0d1ec90a7bbd81898e05dbaf50

renexdev commented 6 years ago

At the end of the given gist, I write these warnings... should I listen to them?, there are similar warning at aragon-core...

npm run test //Detected Warnings

  • apps/finance ,$LOCALWORKSPACE/aragon-apps/apps/finance/contracts/Finance.sol:417:40: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. function getBudget(address _token) transitionsPeriod public view returns (uint256 budget, bool hasBudget, uint256 remainingBudget) { ^---------------^ ,$LOCALWORKSPACE/aragon-apps/apps/finance/contracts/Finance.sol:541:5: Warning: Function state mutability can be restricted to view function _canMakePayment(ERC20 _token, uint256 _amount) internal returns (bool) { ^
  • apps/group //no warnings
  • apps/token-manager //no warnings
  • apps/vault //no warnings
  • apps/voting

,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:133:42: Warning: Unused function parameter. Remove or comment out the variable name to silence this warning. function canForward(address _sender, bytes _evmCallScript) public view returns (bool) { ^------------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:140:16: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. return _isVoteOpen(vote) && token.balanceOfAt(_voter, vote.snapshotBlock) > 0; ^---------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:150:13: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. if (_isValuePct(vote.yea, vote.totalVoters, supportRequiredPct)) ^---------------------------------------------------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:155:27: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. bool voteEnded = !_isVoteOpen(vote); ^---------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:156:27: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. bool hasSupport = _isValuePct(vote.yea, totalVotes, supportRequiredPct); ^---------------------------------------------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:157:29: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. bool hasMinQuorum = _isValuePct(vote.yea, vote.totalVoters, vote.minAcceptQuorumPct); ^--------------------------------------------------------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:165:16: Warning: Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable. open = _isVoteOpen(vote); ^---------------^ ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:257:5: Warning: Function state mutability can be restricted to view function _isVoteOpen(Vote storage vote) internal returns (bool) { ^ Spanning multiple lines. ,$LOCALWORKSPACE/aragon-apps/apps/voting/contracts/Voting.sol:264:5: Warning: Function state mutability can be restricted to pure function _isValuePct(uint256 _value, uint256 _total, uint256 _pct) internal returns (bool) { ^

izqui commented 6 years ago

@renexdev Bounty amounts are taking a bit to show up, but you will soon be able to claim a 10 ANT bounty in https://openbounty.status.im

renexdev commented 6 years ago

Thanks @izqui! I most appreciate how much I'm learning about the devTools and the Aragon ecosystem!