Reserve contest - tnevler's results

Report

Non-Critical Issues

[N-1]: Function defines a named return variable but then it uses return statements

Context:

1. return true; L176
2. return _price(false); L317
3. return _price(true); L326
4. return (issueQueues[account].left, issueQueues[account].right); L635
5. return (false, req); L50
6. return (true, req); L72
7. return super.propose(targets, values, calldatas, description); L92
8. return ( L105
9. return L220
10. return L239
11. return auction.encodedClearingOrder; L111
12. return auction.encodedClearingOrder; L123
13. return super.price(); L52

Recommendation:

Choose named return variable or return statement. It is unnecessary to use both.

[N-2]: Use of immutable instead of constant keccak expression

Context:

1. bytes32 private constant _PERMIT_TYPEHASH = L126
2. bytes32 private constant _DELEGATE_TYPEHASH = L27
3. bytes32 constant UIntOutofBoundsHash = keccak256(abi.encodeWithSignature("UIntOutOfBounds()")); L33
4. bytes32 public constant METADEPOSIT_TYPEHASH = L49
5. bytes32 public constant METAWITHDRAWAL_TYPEHASH = L53

Description:

According to official solidity documentation for a constant variable, the expression assigned to it is copied to all the places where it is accessed and also re-evaluated each time. It is recommended to use immutable instead.

[N-3]: Scientific notation

Context:

1. require(share.rsrDist <= 10000, "RSR distribution too high"); L165
2. require(share.rTokenDist <= 10000, "RToken distribution too high"); L166

Description:

Scientific notation should be used for better code readability.

[N-4]: No same value input check

Context:

1. tradingDelay = val; L259
2. backingBuffer = val; L266
3. auctionLength = newAuctionLength; L139
4. period = period_; L91
5. ratio = ratio_; L100
6. basketsNeeded = basketsNeeded_; L583
7. issuanceRate = val; L592
8. battery.scalingRedemptionRate = val; L605
9. name = name_; L804
10. symbol = symbol_; L808
11. shortFreeze = shortFreeze_; L183
12. longFreeze = longFreeze_; L190

Recommendation:

Example how to fix require(_newOwner != owner, " Same address");

[N-5]: Wrong order of functions

Context:

1. function register(IAsset asset) external governance returns (bool) { L61 (external function can not go after public function)
2. function setTradingDelay(uint48 val) public governance { L256 (public function can not go after private function)
3. function setDisabled(bool disabled_) external governance { L143 (external function can not go after public function)
4. struct Transfer { L67 (struct definition can not go after external function)
5. uint256[47] private __gap; L108 (state variable can not go after public function)
6. function poke() external { L43 (external function can not go after public function)
7. function currentEra() external view returns (uint256) { L55 (external function can not go after internal )
8. uint64 constant FIX_HALF = uint64(FIX_SCALE) / 2; L310 (state variable declaration can not go after internal function)
9. struct Pending { L43 (struct definition can not go after external function)
10. event StRSRSet(IStRSR indexed oldVal, IStRSR indexed newVal); L108 (event definition can not go after external function)
11. event MainInitialized(); L154 (event definition can not go after external function)
12. function grantRole(bytes32 role, address account) L89 (public function can not go after internal function)
13. IRToken public rToken; L34 (state variable declaration can not go after internal function)
14. modifier notPausedOrFrozen() { L41 (modifier definition can not go after internal function)
15. struct BasketRange { L116 (struct definition can not go after external function)
16. function settleTrade(IERC20 sell) external notPausedOrFrozen nonReentrant { L66 (external function can not go after internal function)
17. modifier nonReentrant() { L49 (modifier definition can not go after constructor)
18. function lotPrice() external view virtual returns (uint192 lotLow, uint192 lotHigh) { L126 (external function can not go after public function)
19. function claimRewards() external virtual override { L52 (external function can not go after public function)
20. function claimRewards() external virtual override { L55 (external function can not go after public function)
21. function lotPrice() external view returns (uint192 lotLow, uint192 lotHigh) { L87 (external function can not go after public function)
22. function rate() external view returns (uint256) { L54 (external function can not go after public function)
23. function exchangeRateCurrent() external returns (uint256) { L27 (external function can not go after public function)
24. function getRoundData(uint80 _roundId) L71 (external function can not go after public function)
25. function setSimplyRevert(bool on) external { L27 (external function can not go after public function)

Description:

According to official solidity documentation functions should be grouped according to their visibility and ordered:

• constructor

• receive function (if exists)

• fallback function (if exists)

• external

• public

• internal

• private

Within a grouping, place the view and pure functions last.

Recommendation:

Put the functions in the correct order according to the documentation.

[N-6]: Typos

Context:

1. /// Mointain the overall backing policy; handout assets otherwise L89 (Change Mointain to Maintain)
2. // (trades[addr] == true) iff this contract has created an ITrade clone at addr L47 (Change iff to if)
3. // lastPayoutBal' = rToken.balanceOf'(this) (balance now == at end of pay leriod) L66 (Change leriod to period)
4. // queue.right == left iff there are no more pending issuances in this queue L106 (Change iff to if)
5. // issuances, and so any particular issuance is actually the _difference_ between two adjaacent L109 (Change adjaacent to adjacent)
6. // The way to keep an IssueQueue striaght in your head is to think of each TotalIssue item as a L112 (Change striaght to straight)
7. // We define a (partial) ordering on IssueItems: item1 < item2 iff the following all hold: L124 (Change iff to if)
8. * If this happens, users balances are zereod out and StRSR is re-issued at a 1:1 exchange rate L25 (Change zereod to zeroed)
9. // r'.queue is r.queue with a new entry appeneded for (totalDrafts' - totalDraft) drafts L541 (Change appeneded to appended)
10. /// Overriden in StRSRVotes to handle rebases L570 (Change Overriden to Overridden)
11. // _delegates[account] is the address of the delegate that `accountt` has specified L30 (Change accountt to account)
12. // Every function should revert iff its result is out of bounds. L21 (Change iff to if)
13. bytes32 constant UIntOutofBoundsHash = keccak256(abi.encodeWithSignature("UIntOutOfBounds()")); L33 (Change UIntOutofBoundsHash to UIntOutOfBoundsHash)
14. /// @return backing The worst-case collaterazation % the protocol will have after done trading L100 (Change collaterazation to collateralization???)
15. * @notice A UX-friendly layer for interactin with the protocol L68 (Change interactin to interaction)
16. /// That is, id was cancelled iff firstId <= id < endId L47 (Change iff to if)
17. /// @param redeemer The address of the account redeeeming RTokens L74 (Change redeeeming to redeeming)
18. /// Set the fraction of the RToken supply that can be reedemed at once L178 (Change reedemed to redeemed)
19. /// Emitted whenever RSR are paids out L71 (Change paids to paid)
20. * Typically freezing thaws on its own in a predetemined number of blocks. L22 (Change predetemined to predetermined)
21. // trades[sell] != 0 iff trade[sell] has been opened and not yet settled L37 (Change iff to if)
22. * @return The amound of pending rewards in RAY L524 (Change amound to amount)
23. function setFeeParameters(uint256 newFeeNumerator, address newfeeReceiverAddress) L127 (Change newfeeReceiverAddress to newFeeReceiverAddress)
24. // @dev: function to intiate a new auction L137 (Change intiate** to initiate)
25. "limit price not better than mimimal offer" L281 (Change mimimal to minimal)
26. /// A Gnosis Mock that attemts to reenter on initiateAuction L10 (Change attemts to attempts)
27. function openTrade(TradeRequest memory req) external view notPausedOrFrozen returns (ITrade) { L38 (external function can not go after public function)
28. bytes memory returndata = address(token).functionCall( L198 (Change returndata to returnData)
29. (bool success, bytes memory returndata) = target.call{ value: value }(data); L797 (Change returndata to returnData)
30. (bool success, bytes memory returndata) = target.staticcall(data); L829 (Change returndata to returnData)
31. bytes memory returndata, L835 (Change returndata to returnData)
32. // from this trade's acution will all eventually go to origin. L47 (Change acution to auction)
33. ```// transfer all balancess of buy and sell at this address to `origin```` L166 (Change balancess to balances)

[N-7]: Line is too long

Context:

1. * @dev Claims reward for an user on behalf, on all the assets of the lending pool, accumulating the pending rewards. The caller must L90
2. "Withdraw(address owner,address recipient,uint256 staticAmount,uint256 dynamicAmount,bool toUnderlying,uint256 nonce,uint256 deadline)" L55
3. * @notice Compute the pending in RAY (rounded down). Pending is the amount to add (not yet unclaimed) rewards in RAY (rounded down). L520
4. * Logic was copied and modified from here: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/SafeCast.sol L653

Description:

Maximum suggested line length is 120 characters.