feat: throw semantic errors

Closes #107. Enables kleros/kleros-juror-front#10
kleros · Mar 21, 2018 · 68aa583 · 68aa583
1 parent 7289474
commit 68aa583
Show file tree

Hide file tree

Showing 17 changed files with 422 additions and 465 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,25 +3,21 @@
 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
 
 <a name="0.0.70"></a>
-## [0.0.70](https://github.com/kleros/kleros-api/compare/v0.0.69...v0.0.70) (2018-03-16)
 
+## [0.0.70](https://github.com/kleros/kleros-api/compare/v0.0.69...v0.0.70) (2018-03-16)
 
 ### Bug Fixes
 
 * **notifications:** error in stateful notifications ([2155f60](https://github.com/kleros/kleros-api/commit/2155f60))
 
-
-
 <a name="0.0.69"></a>
-## [0.0.69](https://github.com/kleros/kleros-api/compare/v0.0.68...v0.0.69) (2018-03-15)
 
+## [0.0.69](https://github.com/kleros/kleros-api/compare/v0.0.68...v0.0.69) (2018-03-15)
 
 ### Bug Fixes
 
 * untranspiled files outside src directory ([83f1d59](https://github.com/kleros/kleros-api/commit/83f1d59))
 
-
-
 <a name="0.0.68"></a>
 
 ## [0.0.68](https://github.com/kleros/kleros-api/compare/v0.0.65...v0.0.68) (2018-03-15)

diff --git a/package.json b/package.json
@@ -1,19 +1,12 @@
 {
   "name": "kleros-api",
   "version": "0.0.70",
-  "description": "A Javascript library that makes it easy to build relayers and other DApps that use the Kleros protocol.",
-  "keywords": [
-    "Blockchain",
-    "Ethereum",
-    "Kleros"
-  ],
+  "description":
+    "A Javascript library that makes it easy to build relayers and other DApps that use the Kleros protocol.",
+  "keywords": ["Blockchain", "Ethereum", "Kleros"],
   "main": "lib/index.js",
   "module": "es/index.js",
-  "files": [
-    "es",
-    "lib",
-    "umd"
-  ],
+  "files": ["es", "lib", "umd"],
   "homepage": "https://kleros.io",
   "repository": "github:kleros/kleros-api",
   "bugs": "https://github.com/kleros/kleros-api/issues",
@@ -35,12 +28,11 @@
     "commitmsg": "kleros-scripts commitmsg",
     "cz": "kleros-scripts cz",
     "start": "babel src --out-dir ./es --watch --source-maps",
-    "build": "rimraf ./umd ./es ./lib && webpack --env.NODE_ENV=production -p && babel src --out-dir ./es --source-maps && cross-env BABEL_ENV=commonjs babel src --out-dir ./lib --source-maps"
+    "build":
+      "rimraf ./umd ./es ./lib && webpack --env.NODE_ENV=production -p && babel src --out-dir ./es --source-maps && cross-env BABEL_ENV=commonjs babel src --out-dir ./lib --source-maps"
   },
   "commitlint": {
-    "extends": [
-      "@commitlint/config-conventional"
-    ]
+    "extends": ["@commitlint/config-conventional"]
   },
   "devDependencies": {
     "babel-cli": "^6.26.0",

diff --git a/src/abstractWrappers/AbstractWrapper.js b/src/abstractWrappers/AbstractWrapper.js
@@ -1,3 +1,5 @@
+import * as errorConstants from '../constants/error'
+
 class AbstractWrapper {
   /**
    * AbstractWrapper is the parent class for abstract classes that interact with the
@@ -58,9 +60,7 @@ class AbstractWrapper {
    */
   _checkArbitratorWrappersSet = () => {
     if (!this._Arbitrator)
-      throw new Error(
-        'No Arbitrator Contract Wrapper specified. Please call setArbitrator'
-      )
+      throw new Error(errorConstants.NO_ARBITRATOR_WRAPPER_SPECIFIED)
   }
 
   /**
@@ -69,9 +69,7 @@ class AbstractWrapper {
    */
   _checkArbitrableWrappersSet = () => {
     if (!this._ArbitrableContract)
-      throw new Error(
-        'No Arbitrable Contract Wrapper specified. Please call setArbitrable'
-      )
+      throw new Error(errorConstants.NO_ARBITRABLE_WRAPPER_SPECIFIED)
   }
 
   /**

diff --git a/src/abstractWrappers/Arbitrator.js b/src/abstractWrappers/Arbitrator.js
@@ -34,16 +34,8 @@ class Arbitrator extends AbstractWrapper {
     arbitratorAddress, // address of KlerosPOC
     account
   ) => {
-    const txHash = await this._Arbitrator.buyPNK(
-      amount,
-      arbitratorAddress,
-      account
-    )
-    if (txHash) {
-      return this.getPNKBalance(arbitratorAddress, account)
-    } else {
-      throw new Error('unable to buy PNK')
-    }
+    await this._Arbitrator.buyPNK(amount, arbitratorAddress, account)
+    return this.getPNKBalance(arbitratorAddress, account)
   }
 
   /**

diff --git a/src/abstractWrappers/Disputes.js b/src/abstractWrappers/Disputes.js
@@ -256,50 +256,39 @@ class Disputes extends AbstractWrapper {
    * @param {string} account - Ethereum account.
    * @param {string} arbitrableContractAddress - Address address of arbitrable contract.
    * @param {number} [arbitrationCost=DEFAULT_ARBITRATION_FEE] - Amount to pay the arbitrator.
-   * @returns {string} - txHash hash transaction | Error.
+   * @returns {object} - The result transaction object.
    */
-  raiseDisputePartyA = async (
+  raiseDisputePartyA = (
     account,
     arbitrableContractAddress,
     arbitrationCost = arbitratorConstants.DEFAULT_ARBITRATION_FEE
   ) => {
     this._checkArbitrableWrappersSet()
-    try {
-      const txHash = await this._ArbitrableContract.payArbitrationFeeByPartyA(
-        account,
-        arbitrableContractAddress,
-        arbitrationCost
-      )
-
-      if (!txHash) throw new Error('unable to pay arbitration fee for party A')
-      return txHash
-    } catch (err) {
-      throw new Error(err)
-    }
+    return this._ArbitrableContract.payArbitrationFeeByPartyA(
+      account,
+      arbitrableContractAddress,
+      arbitrationCost
+    )
   }
 
   /**
    * Pay the arbitration fee to raise a dispute. To be called by the party B.
    * @param {string} account - Ethereum account.
    * @param {string} arbitrableContractAddress - Address address of arbitrable contract.
    * @param {number} [arbitrationCost=DEFAULT_ARBITRATION_FEE] - Amount to pay the arbitrator.
-   * @returns {string} - txHash hash of the transaction | Error.
+   * @returns {object} - The result transaction object.
    */
-  raiseDisputePartyB = async (
+  raiseDisputePartyB = (
     account,
     arbitrableContractAddress,
     arbitrationCost = arbitratorConstants.DEFAULT_ARBITRATION_FEE
   ) => {
     this._checkArbitrableWrappersSet()
-
-    const txHash = await this._ArbitrableContract.payArbitrationFeeByPartyB(
+    return this._ArbitrableContract.payArbitrationFeeByPartyB(
       account,
       arbitrableContractAddress,
       arbitrationCost
     )
-
-    if (!txHash) throw new Error('unable to pay arbitration fee for party B')
-    return txHash
   }
 
   /**
@@ -435,30 +424,23 @@ class Disputes extends AbstractWrapper {
    * @param {number} ruling - Int representing the jurors decision.
    * @param {number[]} draws - Int[] of drawn votes for dispute.
    * @param {string} account - Address of user.
-   * @returns {string} - Transaction hash | Error.
+   * @returns {object} - The result transaction object.
    */
-  submitVotesForDispute = async (
+  submitVotesForDispute = (
     arbitratorAddress,
     disputeId,
     ruling,
     draws,
     account
-  ) => {
-    const txHash = await this._Arbitrator.submitVotes(
+  ) =>
+    this._Arbitrator.submitVotes(
       arbitratorAddress,
       disputeId,
       ruling,
       draws,
       account
     )
 
-    if (txHash) {
-      return txHash
-    } else {
-      throw new Error('unable to submit votes')
-    }
-  }
-
   /**
    * Gets the deadline for an arbitrator's period, which is also the deadline for all its disputes.
    * @param {string} arbitratorAddress - The address of the arbitrator contract.
@@ -580,7 +562,7 @@ class Disputes extends AbstractWrapper {
     )
 
     if (_.isEmpty(disputeArray))
-      throw new Error(`User ${account} does not have store data for dispute`)
+      throw new Error(errorConstants.NO_STORE_DATA_FOR_DISPUTE(account))
 
     return disputeArray[0]
   }
@@ -630,17 +612,10 @@ class Disputes extends AbstractWrapper {
    * @returns {object[]} - Array of ruling objects.
    */
   getRulingOptions = async (arbitratorAddress, disputeId) => {
-    const dispute = await this._Arbitrator.getDispute(
+    const arbitrableContractAddress = (await this._Arbitrator.getDispute(
       arbitratorAddress,
       disputeId
-    )
-    if (!dispute) {
-      throw new Error(
-        `Cannot fetch ruling options: Dispute from arbitrator ${arbitratorAddress} with disputeId: ${disputeId} does not exist`
-      )
-    }
-    const arbitrableContractAddress = dispute.arbitratedContract
-
+    )).arbitratedContract
     return this._ArbitrableContract.getRulingOptions(
       arbitrableContractAddress,
       arbitratorAddress,
@@ -660,16 +635,11 @@ class Disputes extends AbstractWrapper {
     this._checkArbitratorWrappersSet()
     this._checkArbitrableWrappersSet()
 
-    // Get dispute data from contract, and throw if not found. Also get current session and period
+    // Get dispute data from contract. Also get the current session and period.
     const [dispute, arbitratorData] = await Promise.all([
       this._Arbitrator.getDispute(arbitratorAddress, disputeId),
       this._Arbitrator.getData(arbitratorAddress, account)
     ])
-    if (!dispute) {
-      throw new Error(
-        `Dispute with arbitrator: ${arbitratorAddress} and disputeId: ${disputeId} does not exist`
-      )
-    }
 
     // Get arbitrable contract data and evidence
     const arbitrableContractAddress = dispute.arbitratedContract
@@ -802,43 +772,36 @@ class Disputes extends AbstractWrapper {
    * @returns {int[]} - array of active disputeId
    */
   _getOpenDisputesForSession = async arbitratorAddress => {
+    const currentSession = (await this._Arbitrator.getData(arbitratorAddress))
+      .session
     const openDisputes = []
-    // contract data
-    const arbitratorData = await this._Arbitrator.getData(arbitratorAddress)
-    let disputeId = 0
-    const currentSession = arbitratorData.session
 
+    let disputeId = 0
     let dispute
     while (1) {
-      // iterate over all disputes (FIXME inefficient)
+      // Iterate over all the disputes
+      // TODO: Implement a more performant solution
       try {
-        try {
-          dispute = await this._Arbitrator.getDispute(
-            arbitratorAddress,
-            disputeId
-          )
-          // eslint-disable-next-line no-unused-vars
-        } catch (err) {
-          // FIXME standardize
-          throw new Error(errorConstants.TYPE.DISPUTE_OUT_OF_RANGE)
-        }
-
-        if (dispute.arbitratedContract === ethConstants.NULL_ADDRESS) break
-        // session + number of appeals
-        const disputeSession = dispute.firstSession + dispute.numberOfAppeals
-        // if dispute not in current session skip
-        if (disputeSession !== currentSession) {
-          disputeId++
-          continue
-        }
-
-        openDisputes.push(disputeId)
-        // check next dispute
-        disputeId += 1
+        dispute = await this._Arbitrator.getDispute(
+          arbitratorAddress,
+          disputeId
+        )
       } catch (err) {
-        if (err.message === errorConstants.TYPE.DISPUTE_OUT_OF_RANGE) break
+        // Dispute out of range, break
+        if (err.message === errorConstants.UNABLE_TO_FETCH_DISPUTE) break
+        console.error(err)
         throw err
       }
+
+      // Dispute has no arbitrable contract, break
+      if (dispute.arbitratedContract === ethConstants.NULL_ADDRESS) break
+
+      // If dispute is in the current session, add it to the result array
+      if (dispute.firstSession + dispute.numberOfAppeals === currentSession)
+        openDisputes.push(disputeId)
+
+      // Advance to the next dispute
+      disputeId++
     }
 
     return openDisputes

diff --git a/src/abstractWrappers/Notifications.js b/src/abstractWrappers/Notifications.js
@@ -4,6 +4,7 @@ import * as ethConstants from '../constants/eth'
 import * as arbitratorConstants from '../constants/arbitrator'
 import * as notificationConstants from '../constants/notification'
 import * as disputeConstants from '../constants/dispute'
+import * as errorConstants from '../constants/error'
 
 import AbstractWrapper from './AbstractWrapper'
 
@@ -310,12 +311,14 @@ class Notifications extends AbstractWrapper {
 
             await this._sendPushNotification(callback, notification)
           }
-          // check next dispute
-          disputeId += 1
-          // eslint-disable-next-line no-unused-vars
+
+          // Check the next dispute
+          disputeId++
         } catch (err) {
-          // getDispute(n) throws an error if index out of range
-          break
+          // Dispute out of range, break
+          if (err.message === errorConstants.UNABLE_TO_FETCH_DISPUTE) break
+          console.error(err)
+          throw err
         }
       }
     }