refactor: code review updates

.travis.yml

@@ -12,7 +12,7 @@ addons:
 install: yarn install --pure-lockfile
 script:
   - yarn run lint
-  - nohup yarn run ganache-cli -a 15 &
+  - nohup yarn run ganache &
   - yarn test
   - if [ -n "$COVERALLS_REPO_TOKEN" ]; then yarn run test:coveralls; fi
   - yarn run build

package.json

@@ -22,14 +22,14 @@
   "scripts": {
     "prettify": "kleros-scripts prettify",
     "lint": "kleros-scripts lint:js --config ./.eslintrc.js",
+    "ganache": "ganache-cli -a 15",
     "test": "jest --config ./jest.config.js",
     "test:coveralls": "coveralls < ./coverage/lcov.info",
     "precommit": "kleros-scripts precommit",
     "commitmsg": "kleros-scripts commitmsg",
     "cz": "kleros-scripts cz",
     "start": "webpack --env.NODE_ENV=development --progress --watch",
-    "build": "webpack --env.NODE_ENV=production -p",
-    "ganache": "ganache-cli -a 15"
+    "build": "webpack --env.NODE_ENV=production -p"
   },
   "commitlint": {
     "extends": [

src/abstractWrappers/Disputes.js

@@ -94,7 +94,7 @@ class Disputes extends AbstractWrapper {
           dispute.arbitratorAddress,
           dispute.disputeId,
           {
-            netPNK: (dispute.netPNK ? dispute.netPNK : 0) + amountShift
+            netPNK: (dispute.netPNK || 0) + amountShift
           }
         )
       }
@@ -513,19 +513,21 @@ class Disputes extends AbstractWrapper {
 
     // update dispute
     const dispute = await this._StoreProvider.updateDispute(
-      disputeData.disputeId,
       disputeData.arbitratorAddress,
-      disputeData.arbitrableContractAddress,
-      disputeData.partyA,
-      disputeData.partyB,
-      disputeData.title,
-      disputeData.status,
-      disputeData.information,
-      disputeData.justification,
-      disputeData.resolutionOptions,
-      disputeData.appealCreatedAt,
-      disputeData.appealRuledAt,
-      disputeData.appealDeadlines
+      disputeData.disputeId,
+      {
+        contractAddress: disputeData.arbitrableContractAddress,
+        partyA: disputeData.partyA,
+        partyB: disputeData.partyB,
+        title: disputeData.title,
+        status: disputeData.status,
+        information: disputeData.information,
+        justification: disputeData.justification,
+        resolutionOptions: disputeData.resolutionOptions,
+        appealCreatedAt: disputeData.appealCreatedAt,
+        appealRuledAt: disputeData.appealRuledAt,
+        appealDeadlines: disputeData.appealDeadlines
+      }
     )
 
     const storedDisputeData = await this._StoreProvider.getDisputeData(

src/abstractWrappers/Notifications.js

@@ -223,7 +223,7 @@ class Notifications extends AbstractWrapper {
    * @param {number} logIndex index of the log. used to differentiate logs if multiple logs per tx
    * @returns {promise} promise that can be waited on for syncronousity
    */
-  markNotificationAsRead = async (account, txHash, logIndex) =>
+  markNotificationAsRead = (account, txHash, logIndex) =>
     this._StoreProvider.markNotificationAsRead(account, txHash, logIndex, true)
 
   /**

src/utils/StoreProviderWrapper.js

@@ -140,7 +140,7 @@ class StoreProviderWrapper {
   getLastBlock = async account => {
     const userProfile = await this.getUserProfile(account)
 
-    return userProfile.lastBlock ? userProfile.lastBlock : 0
+    return userProfile.lastBlock || 0
   }
 
   getDispute = async (arbitratorAddress, disputeId) => {
@@ -157,7 +157,7 @@ class StoreProviderWrapper {
   // **************************** //
 
   resetUserProfile = async account => {
-    const getBodyFn = async () =>
+    const getBodyFn = () =>
       new Promise(resolve =>
         resolve(
           JSON.stringify({
@@ -179,22 +179,15 @@ class StoreProviderWrapper {
    * @param {object} params object containing kwargs to update
    * @returns {promise} resulting profile
    */
-  updateUserProfile = async (account, params = {}) => {
+  updateUserProfile = (account, params = {}) => {
     const getBodyFn = async () => {
       const currentProfile = (await this.getUserProfile(account)) || {}
+      delete currentProfile._id
+      delete currentProfile.created_at
 
-      return new Promise(resolve => {
-        resolve(
-          JSON.stringify({
-            address: account,
-            session: params.session || currentProfile.session,
-            lastBlock: params.session || currentProfile.session,
-            contracts: params.contracts || currentProfile.contracts, // DANGER use contract updating methods
-            disputes: params.disputes || currentProfile.disputes, // DANGER user dispute updating methods
-            notifications: params.notifications || currentProfile.notifications // DANGER user notification updating methods
-          })
-        )
-      })
+      params.address = account
+
+      return JSON.stringify({ ...currentProfile, ...params })
     }
 
     return this.queueWriteRequest(
@@ -219,30 +212,18 @@ class StoreProviderWrapper {
     return userProfile
   }
 
-  updateContract = async (account, address, params) => {
+  updateContract = (account, address, params) => {
     const getBodyFn = async () => {
       let currentContractData = await this.getContractByAddress(
         account,
         address
       )
       if (!currentContractData) currentContractData = {}
+      delete currentContractData._id
 
-      return new Promise(resolve =>
-        resolve(
-          JSON.stringify({
-            address: address || currentContractData.address,
-            hashContract: params.hashContract || currentContractData.hash,
-            partyA: params.partyA || currentContractData.partyA,
-            partyB: params.partyB || currentContractData.partyB,
-            arbitrator: params.arbitrator || currentContractData.arbitrator,
-            timeout: params.timeout || currentContractData.timeout,
-            email: params.email || currentContractData.email,
-            title: params.title || currentContractData.title,
-            description: params.description || currentContractData.description,
-            disputeId: params.disputeId || currentContractData.disputeId
-          })
-        )
-      )
+      params.address = address
+
+      return JSON.stringify({ ...currentContractData, ...params })
     }
 
     return this.queueWriteRequest(
@@ -252,7 +233,7 @@ class StoreProviderWrapper {
     )
   }
 
-  addEvidenceContract = async (address, account, name, description, url) => {
+  addEvidenceContract = (address, account, name, description, url) => {
     // get timestamp for submission
     const submittedAt = new Date().getTime()
 
@@ -275,12 +256,7 @@ class StoreProviderWrapper {
     )
   }
 
-  updateDisputeProfile = async (
-    account,
-    arbitratorAddress,
-    disputeId,
-    params
-  ) => {
+  updateDisputeProfile = (account, arbitratorAddress, disputeId, params) => {
     const getBodyFn = async () => {
       const userProfile = await this.getUserProfile(account)
 
@@ -292,18 +268,12 @@ class StoreProviderWrapper {
       )
 
       const currentDisputeProfile = userProfile.disputes[disputeIndex] || {}
+      delete currentDisputeProfile._id
+      // set these so if it is a new dispute they are included
+      params.disputeId = disputeId
+      params.arbitratorAddress = arbitratorAddress
 
-      return new Promise(resolve =>
-        resolve(
-          JSON.stringify({
-            arbitratorAddress: arbitratorAddress,
-            disputeId: disputeId,
-            appealDraws:
-              params.appealDraws || currentDisputeProfile.appealDraws,
-            netPNK: params.netPNK || currentDisputeProfile.netPNK
-          })
-        )
-      )
+      return JSON.stringify({ ...currentDisputeProfile, ...params })
     }
 
     return this.queueWriteRequest(
@@ -319,31 +289,13 @@ class StoreProviderWrapper {
     const getBodyFn = async () => {
       const currentDispute =
         (await this.getDispute(arbitratorAddress, disputeId)) || {}
+      delete currentDispute._id
+      delete currentDispute.updated_at
 
-      return new Promise(resolve =>
-        resolve(
-          JSON.stringify({
-            disputeId,
-            arbitratorAddress,
-            arbitrableContractAddress:
-              params.arbitrableContractAddress ||
-              currentDispute.arbitrableContractAddress,
-            partyA: params.partyA || currentDispute.partyA,
-            partyB: params.partyB || currentDispute.partyB,
-            title: params.title || currentDispute.title,
-            status: params.status || currentDispute.status,
-            information: params.information || currentDispute.information,
-            justification: params.justification || currentDispute.justification,
-            resolutionOptions:
-              params.resolutionOptions || currentDispute.resolutionOptions,
-            appealCreatedAt:
-              params.appealCreatedAt || currentDispute.appealCreatedAt,
-            appealRuledAt: params.appealRuledAt || currentDispute.appealRuledAt,
-            appealDeadlines:
-              params.appealDeadlines || currentDispute.appealDeadlines
-          })
-        )
-      )
+      params.arbitratorAddress = arbitratorAddress
+      params.disputeId = disputeId
+
+      return JSON.stringify({ ...currentDispute, ...params })
     }
 
     return this.queueWriteRequest(
@@ -362,7 +314,7 @@ class StoreProviderWrapper {
     data = {},
     read = false
   ) => {
-    const getBodyFn = async () =>
+    const getBodyFn = () =>
       new Promise(resolve =>
         resolve(
           JSON.stringify({
@@ -399,7 +351,7 @@ class StoreProviderWrapper {
       userProfile.notifications[notificationIndex].read = isRead
       delete userProfile._id
       delete userProfile.created_at
-      return new Promise(resolve => resolve(JSON.stringify(userProfile)))
+      return JSON.stringify(userProfile)
     }
 
     return this.queueWriteRequest(

tests/integration/disputeResolution.test.js

@@ -135,7 +135,6 @@ describe('Dispute Resolution', () => {
       expect(juror1Data[4].toNumber() - juror1Data[3].toNumber()).toEqual(
         parseInt(web3.toWei(activatedTokenAmount, 'ether'), 10)
       )
-
       // return a bigint
       // FIXME use arbitrableTransaction
       const arbitrableContractInstance = await KlerosInstance.arbitrableTransaction.load(
@@ -183,7 +182,6 @@ describe('Dispute Resolution', () => {
       expect(txHashRaiseDisputeByPartyB).toEqual(
         expect.stringMatching(/^0x[a-f0-9]{64}$/)
       ) // tx hash
-
       const dispute = await KlerosInstance.klerosPOC.getDispute(
         klerosPOCAddress,
         0
@@ -205,7 +203,6 @@ describe('Dispute Resolution', () => {
         0
       )
       expect(resolutionOptions.length).toEqual(2)
-
       // add an evidence for partyA
       const testName = 'test name'
       const testDesc = 'test description'
@@ -230,7 +227,6 @@ describe('Dispute Resolution', () => {
         arbitrableContractAddress,
         partyA
       )
-
       expect(contractStoreData.evidences[0].url).toBe(testURL)
       expect(contractStoreData.evidences[0].submittedAt).toBeTruthy()
 
@@ -321,7 +317,6 @@ describe('Dispute Resolution', () => {
         drawB,
         juror2
       )
-
       const winningRuling =
         drawA.length > drawB.length ? rulingJuror1 : rulingJuror2
 
@@ -346,7 +341,6 @@ describe('Dispute Resolution', () => {
         0,
         other
       )
-
       // execute ruling
       await KlerosInstance.klerosPOC.executeRuling(klerosPOCAddress, 0, other)
       // balances after ruling
@@ -375,6 +369,6 @@ describe('Dispute Resolution', () => {
         klerosPOCAddress
       )
     },
-    80000
+    100000
   )
 })

tests/integration/notifications.test.js

@@ -458,7 +458,6 @@ describe('Notifications and Event Listeners', () => {
       KlerosInstance.eventListener.stopWatchingArbitratorEvents(
         klerosPOCAddress
       )
-
       // TODO find way to check timestamps and other non-callback notifications
     },
     200000

util/PromiseQueue.js

@@ -7,12 +7,15 @@ const PromiseQueue = () => {
     },
     fetch: fn => {
       let returnResolver
-      const returnPromise = new Promise(resolve => {
+      let returnRejecter
+      const returnPromise = new Promise((resolve, reject) => {
         returnResolver = resolve
+        returnRejecter = reject
       })
-      promise = promise.then(fn, fn).then(result => {
-        returnResolver(result)
-      })
+      promise = promise
+        .then(fn, fn)
+        .then(res => returnResolver(res), err => returnRejecter(err))
+
       return returnPromise
     }
   }