Court: Polish main court contract functionality

.travis.yml

@@ -6,8 +6,28 @@ notifications:
   email: false
 node_js:
   - '8'
-env:
-  - TASK=test
 before_script:
   - npm prune
-script: "npm run $TASK"
+jobs:
+  include:
+    - stage: tests
+      script: npm test test/court/court-{initialize,accounting,terms,draft}.js
+      name: "Court Basics"
+    - stage: tests
+      script: npm test test/court/court-{disputes,voting,appeal,confirm-appeal}.js
+      name: "Court Disputes"
+    - stage: tests
+      script: npm test test/court/court-settle.js
+      name: "Court Settles"
+    - stage: tests
+      script: npm test test/registry/*.js
+      name: "Registry"
+    - stage: tests
+      script: npm test test/tree/*.js
+      name: "Tree"
+    - stage: tests
+      script: npm test test/voting/*.js
+      name: "Voting"
+    - stage: tests
+      script: npm test test/*.js
+      name: "Others"

contracts/Agreement.sol

@@ -1,20 +1,17 @@
-pragma solidity ^0.4.15;
+pragma solidity ^0.4.24;
 
 import "./standards/arbitration/Arbitrable.sol";
 
 
 contract Agreement is Arbitrable { /* AragonApp/Trigger */
-    address[] parties;
+    address[] internal parties;
 
     // TODO: Probably needs to be moved into an 'initialize()' function at some point
-    constructor(address _court, address[] _parties)
-        public
-        Arbitrable(_court) {
-
+    constructor(address _court, address[] _parties) public Arbitrable(_court) {
         parties = _parties;
     }
 
-    function canSubmitEvidence(uint256 _disputeId, address _submitter) public view returns (bool) {
+    function canSubmitEvidence(uint256 /* _disputeId */, address /* _submitter */) public view returns (bool) {
         // TODO: should check court to see whether evidence can be submitted for this particular dispute at this point
         uint256 partiesLength = parties.length;
         for (uint256 i = 0; i < partiesLength; i++) {

contracts/CRVoting.sol

@@ -280,7 +280,7 @@ contract CRVoting is Initializable, ICRVoting {
     */
     function _ensureVoterWeightToCommit(uint256 _voteId, address _voter) internal returns (uint256) {
         uint256 weight = uint256(owner.getVoterWeightToCommit(_voteId, _voter));
-        require(weight > uint256(0), ERROR_COMMIT_DENIED_BY_OWNER);
+        require(weight > 0, ERROR_COMMIT_DENIED_BY_OWNER);
         return weight;
     }
 
@@ -292,7 +292,7 @@ contract CRVoting is Initializable, ICRVoting {
     */
     function _ensureVoterWeightToReveal(uint256 _voteId, address _voter) internal returns (uint256) {
         uint256 weight = uint256(owner.getVoterWeightToReveal(_voteId, _voter));
-        require(weight > uint256(0), ERROR_REVEAL_DENIED_BY_OWNER);
+        require(weight > 0, ERROR_REVEAL_DENIED_BY_OWNER);
         return weight;
     }
 

contracts/CourtAccounting.sol

@@ -35,8 +35,7 @@ contract CourtAccounting is IAccounting {
     }
 
     function assign(ERC20 _token, address _to, uint256 _amount) external onlyOwner {
-        // TODO: uncomment, we are testing with 0 fees for now
-        // require(_amount > 0, ERROR_DEPOSIT_AMOUNT_ZERO);
+        require(_amount > 0, ERROR_DEPOSIT_AMOUNT_ZERO);
 
         balances[_token][_to] = balances[_token][_to].add(_amount);
         emit Assign(address(_token), msg.sender, _to, _amount);

contracts/CourtSubscriptions.sol

@@ -6,6 +6,7 @@ import "@aragon/os/contracts/common/IsContract.sol";
 import "@aragon/os/contracts/lib/math/SafeMath.sol";
 import "@aragon/os/contracts/common/TimeHelpers.sol";
 
+import "./lib/PctHelpers.sol";
 import "./standards/erc900/IJurorsRegistry.sol";
 import "./standards/subscription/ISubscriptions.sol";
 import "./standards/subscription/ISubscriptionsOwner.sol";
@@ -14,8 +15,8 @@ import "./standards/subscription/ISubscriptionsOwner.sol";
 contract CourtSubscriptions is IsContract, ISubscriptions, TimeHelpers {
     using SafeERC20 for ERC20;
     using SafeMath for uint256;
+    using PctHelpers for uint256;
 
-    uint256 internal constant PCT_BASE = 10000; // ‱
     uint64 internal constant START_TERM_ID = 1; // term 0 is for jurors onboarding
 
     string internal constant ERROR_NOT_GOVERNOR = "SUB_NOT_GOVERNOR";
@@ -129,7 +130,7 @@ contract CourtSubscriptions is IsContract, ISubscriptions, TimeHelpers {
 
         // governor fee
         // as _periods and feeAmount are > 0, amountToPay will be > 0 and newLastPeriod > subscriber.lastPaymentPeriodId
-        uint256 governorFee = _pct4(amountToPay, governorSharePct);
+        uint256 governorFee = amountToPay.pct(governorSharePct);
         accumulatedGovernorFees += governorFee;
 
         // amount collected for the current period to share among jurors
@@ -352,7 +353,7 @@ contract CourtSubscriptions is IsContract, ISubscriptions, TimeHelpers {
     }
 
     function _setGovernorSharePct(uint16 _governorSharePct) internal {
-        require(_governorSharePct <= PCT_BASE, ERROR_OVERFLOW);
+        require(_governorSharePct <= PctHelpers.base(), ERROR_OVERFLOW);
         governorSharePct = _governorSharePct;
     }
 
@@ -443,7 +444,7 @@ contract CourtSubscriptions is IsContract, ISubscriptions, TimeHelpers {
         require(newLastPeriodId <= _currentPeriodId || newLastPeriodId.sub(_currentPeriodId) < prePaymentPeriods, ERROR_TOO_MANY_PERIODS);
 
         // delayedPeriods * _feeAmount * (1 +  latePaymentPenaltyPct/PCT_BASE) + regularPeriods * _feeAmount
-        amountToPay = _pct4Increase(delayedPeriods.mul(_feeAmount), latePaymentPenaltyPct).add(regularPeriods.mul(_feeAmount));
+        amountToPay = delayedPeriods.mul(_feeAmount).pctIncrease(latePaymentPenaltyPct).add(regularPeriods.mul(_feeAmount));
     }
 
     function _getPeriodBalanceDetails(uint256 _periodId) internal view returns (uint64 periodBalanceCheckpoint, uint256 totalActiveBalance) {
@@ -485,12 +486,4 @@ contract CourtSubscriptions is IsContract, ISubscriptions, TimeHelpers {
         // juror fee share
         return _period.collectedFees.mul(jurorActiveBalance) / _totalActiveBalance;
     }
-
-    function _pct4(uint256 _number, uint16 _pct) internal pure returns (uint256) {
-        return _number.mul(uint256(_pct)) / PCT_BASE;
-    }
-
-    function _pct4Increase(uint256 _number, uint16 _pct) internal pure returns (uint256) {
-        return _number.mul((PCT_BASE + uint256(_pct))) / PCT_BASE;
-    }
 }

contracts/JurorsRegistry.sol

@@ -9,6 +9,7 @@ import { ApproveAndCallFallBack } from "@aragon/apps-shared-minime/contracts/Min
 
 import "./lib/BytesHelpers.sol";
 import "./lib/HexSumTree.sol";
+import "./lib/PctHelpers.sol";
 import "./lib/JurorsTreeSortition.sol";
 import "./standards/erc900/ERC900.sol";
 import "./standards/erc900/IJurorsRegistry.sol";
@@ -18,6 +19,7 @@ import "./standards/erc900/IJurorsRegistryOwner.sol";
 contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, ApproveAndCallFallBack {
     using SafeERC20 for ERC20;
     using SafeMath for uint256;
+    using PctHelpers for uint256;
     using BytesHelpers for bytes;
     using HexSumTree for HexSumTree.Tree;
     using JurorsTreeSortition for HexSumTree.Tree;
@@ -35,7 +37,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     string internal constant ERROR_TOKEN_TRANSFER_FAILED = "JR_TOKEN_TRANSFER_FAILED";
     string internal constant ERROR_TOKEN_APPROVE_NOT_ALLOWED = "JR_TOKEN_APPROVE_NOT_ALLOWED";
 
-    uint256 internal constant PCT_BASE = 10000; // ‱ = 1 / 10,000
+    // Address that will be used to burn juror tokens
     address internal constant BURN_ACCOUNT = 0xdead;
 
     /**
@@ -133,7 +135,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
         _processDeactivationRequest(msg.sender, termId);
 
         uint256 availableBalance = jurorsByAddress[msg.sender].availableBalance;
-        uint256 amountToActivate = _amount == uint256(0) ? availableBalance : _amount;
+        uint256 amountToActivate = _amount == 0 ? availableBalance : _amount;
         require(amountToActivate > 0, ERROR_INVALID_ZERO_AMOUNT);
         require(amountToActivate <= availableBalance, ERROR_INVALID_ACTIVATION_AMOUNT);
 
@@ -148,13 +150,13 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
         uint64 termId = owner.ensureAndGetTermId();
 
         uint256 unlockedActiveBalance = unlockedActiveBalanceOf(msg.sender);
-        uint256 amountToDeactivate = _amount == uint256(0) ? unlockedActiveBalance : _amount;
+        uint256 amountToDeactivate = _amount == 0 ? unlockedActiveBalance : _amount;
         require(amountToDeactivate > 0, ERROR_INVALID_ZERO_AMOUNT);
         require(amountToDeactivate <= unlockedActiveBalance, ERROR_INVALID_DEACTIVATION_AMOUNT);
 
         // No need to use SafeMath here, we already checked values above
         uint256 futureActiveBalance = unlockedActiveBalance - amountToDeactivate;
-        require(futureActiveBalance == uint256(0) || futureActiveBalance >= minActiveBalance, ERROR_INVALID_DEACTIVATION_AMOUNT);
+        require(futureActiveBalance == 0 || futureActiveBalance >= minActiveBalance, ERROR_INVALID_DEACTIVATION_AMOUNT);
 
         _createDeactivationRequest(msg.sender, termId, amountToDeactivate);
     }
@@ -213,7 +215,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     *        3. uint256 Number of seats already filled
     *        4. uint256 Number of seats left to be filled
     *        5. uint64  Number of jurors required for the draft
-    *        6. uint16  Percentage of the minimum active balance to be locked for the draft
+    *        6. uint16  Per ten thousand of the minimum active balance to be locked for the draft
     *
     * @return jurors List of jurors selected for the draft
     * @return weights List of weights corresponding to each juror
@@ -323,7 +325,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     * @return True if the juror has enough unlocked tokens to be collected for the requested term, false otherwise
     */
     function collectTokens(address _juror, uint256 _amount, uint64 _termId) external onlyOwner returns (bool) {
-        if (_amount == uint256(0)) {
+        if (_amount == 0) {
             return true;
         }
 
@@ -345,7 +347,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
             // Note there's no need to use SafeMath here, amounts were already checked above
             uint256 amountToReduce = _amount - unlockedActiveBalance;
             _reduceDeactivationRequest(_juror, amountToReduce, _termId);
-            tree.set(juror.id, nextTermId, uint256(0));
+            tree.set(juror.id, nextTermId, 0);
         } else {
             tree.update(juror.id, nextTermId, _amount, false);
         }
@@ -408,7 +410,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     */
     function activeBalanceOfAt(address _juror, uint64 _termId) external view returns (uint256) {
         Juror storage juror = jurorsByAddress[_juror];
-        return _existsJuror(juror) ? tree.getItemAt(juror.id, _termId) : uint256(0);
+        return _existsJuror(juror) ? tree.getItemAt(juror.id, _termId) : 0;
     }
 
     /**
@@ -446,11 +448,12 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     * @return active Amount of active tokens of a juror
     * @return available Amount of available tokens of a juror
     * @return locked Amount of active tokens that are locked due to ongoing disputes
+    * @return pendingDeactivation Amount of active tokens that were requested for deactivation
     */
     function balanceOf(address _juror) public view returns (uint256 active, uint256 available, uint256 locked, uint256 pendingDeactivation) {
         Juror storage juror = jurorsByAddress[_juror];
 
-        active = _existsJuror(juror) ? tree.getItem(juror.id) : uint256(0);
+        active = _existsJuror(juror) ? tree.getItem(juror.id) : 0;
         available = juror.availableBalance;
         locked = juror.lockedBalance;
         pendingDeactivation = juror.deactivationRequest.amount;
@@ -547,7 +550,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
         // Note that we can use a zeroed term id to denote void here since we are storing
         // the minimum allowed term to deactivate tokens which will always be at least 1
         request.availableTermId = uint64(0);
-        request.amount = uint256(0);
+        request.amount = 0;
         _updateAvailableBalanceOf(_juror, deactivationAmount, true);
 
         emit JurorDeactivationProcessed(_juror, deactivationAvailableTermId, deactivationAmount, _termId);
@@ -644,7 +647,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
         // We are not using a require here to avoid reverting in case any of the accounting maths reaches this point
         // with a zeroed amount value. Instead, we are doing this validation in the external entry points such as
         // stake, unstake, activate, deactivate, among others.
-        if (_amount == uint256(0)) {
+        if (_amount == 0) {
             return;
         }
 
@@ -665,7 +668,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     * @return Amount of active tokens of a juror that are not locked due to ongoing disputes
     */
     function _unlockedActiveBalanceOf(Juror storage _juror) internal view returns (uint256) {
-        return _existsJuror(_juror) ? tree.getItem(_juror.id).sub(_juror.lockedBalance) : uint256(0);
+        return _existsJuror(_juror) ? tree.getItem(_juror.id).sub(_juror.lockedBalance) : 0;
     }
 
     /**
@@ -674,7 +677,7 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     * @return True if the given juror was already registered, false otherwise
     */
     function _existsJuror(Juror storage _juror) internal view returns (bool) {
-        return _juror.id != uint256(0);
+        return _juror.id != 0;
     }
 
     /**
@@ -685,16 +688,16 @@ contract JurorsRegistry is Initializable, IsContract, IJurorsRegistry, ERC900, A
     */
     function _deactivationRequestedAmountForTerm(Juror storage _juror, uint64 _termId) internal view returns (uint256) {
         DeactivationRequest storage request = _juror.deactivationRequest;
-        return request.availableTermId == _termId ? request.amount : uint256(0);
+        return request.availableTermId == _termId ? request.amount : 0;
     }
 
     /**
     * @dev Internal function to tell the fraction of minimum active tokens that must be locked for a draft
-    * @param _pct Percentage of the minimum active balance to be locked for a draft
+    * @param _pct Per ten thousand of the minimum active balance to be locked for a draft
     * @return The fraction of minimum active tokens that must be locked for a draft
     */
     function _draftLockAmount(uint16 _pct) internal view returns (uint256) {
-        return minActiveBalance * uint256(_pct) / PCT_BASE;
+        return minActiveBalance.pct(_pct);
     }
 
     /**

contracts/lib/Checkpointing.sol

@@ -50,11 +50,11 @@ library Checkpointing {
     */
     function getLast(History storage self) internal view returns (uint256) {
         uint256 length = self.history.length;
-        if (length > uint256(0)) {
+        if (length > 0) {
             return uint256(self.history[length - 1].value);
         }
 
-        return uint256(0);
+        return 0;
     }
 
     /**
@@ -89,7 +89,7 @@ library Checkpointing {
     */
     function _add192(History storage self, uint64 _time, uint192 _value) private {
         uint256 length = self.history.length;
-        if (length == uint256(0) || self.history[self.history.length - 1].time < _time) {
+        if (length == 0 || self.history[self.history.length - 1].time < _time) {
             // If there was no value registered or the given point in time is after the latest registered value,
             // we can insert it to the history directly
             self.history.push(Checkpoint(_time, _value));
@@ -113,8 +113,8 @@ library Checkpointing {
     function _backwardsLinearSearch(History storage self, uint64 _time) private view returns (uint256) {
         // If there was no value registered for the given history return simply zero
         uint256 length = self.history.length;
-        if (length == uint256(0)) {
-            return uint256(0);
+        if (length == 0) {
+            return 0;
         }
 
         uint256 index = length - 1;
@@ -124,7 +124,7 @@ library Checkpointing {
             checkpoint = self.history[index];
         }
 
-        return checkpoint.time > _time ? uint256(0) : uint256(checkpoint.value);
+        return checkpoint.time > _time ? 0 : uint256(checkpoint.value);
     }
 
     /**
@@ -138,13 +138,13 @@ library Checkpointing {
     function _binarySearch(History storage self, uint64 _time) private view returns (uint256) {
         // If there was no value registered for the given history return simply zero
         uint256 length = self.history.length;
-        if (length == uint256(0)) {
-            return uint256(0);
+        if (length == 0) {
+            return 0;
         }
 
         // If the requested time is previous to the first registered value, return zero to denote missing checkpoint
         if (_time < self.history[0].time) {
-            return uint256(0);
+            return 0;
         }
 
         // Execute a binary search between the checkpointed times of the history