Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
izqui opened this issue Jan 8, 2018 · 10 comments
Closed

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

izqui opened this issue Jan 8, 2018 · 10 comments

Comments

@izqui
Copy link
Contributor

izqui commented Jan 8, 2018

Similar to aragon/aragonOS#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
Copy link

status-open-bounty commented Jan 10, 2018

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
Copy link
Contributor

renexdev commented Jan 11, 2018

Coding & learning to solve this!

@izqui
Copy link
Contributor Author

izqui commented Jan 11, 2018

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

@renexdev
Copy link
Contributor

I'm following that! Thanks

@renexdev
Copy link
Contributor

@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
Copy link
Contributor Author

izqui commented Jan 11, 2018

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

@renexdev
Copy link
Contributor

renexdev commented Jan 12, 2018

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
Copy link
Contributor

renexdev commented Jan 12, 2018

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) {
^

  • future-apps/fundraising
    //No test provided

@izqui
Copy link
Contributor Author

izqui commented Jan 12, 2018

@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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants