Contracts: Use test helpers (#608)

* Use @aragon/contract-helpers-test * fixup! Use @aragon/contract-helpers-test * wip: Upgrade to truffle v5 * Tests: Adopt test helpers Co-authored-by: Facu Spagnuolo <[email protected]>
aragon · Aug 10, 2020 · 6ba3801 · 6ba3801
1 parent 800dc3e
commit 6ba3801
Show file tree

Hide file tree

Showing 40 changed files with 392 additions and 722 deletions.
diff --git a/package.json b/package.json
@@ -15,12 +15,12 @@
   ],
   "scripts": {
     "compile": "truffle compile",
-    "test": "TRUFFLE_TEST=true npm run ganache-cli:test",
+    "lint": "solium --dir ./contracts",
+    "test": "TRUFFLE_TEST=true npm run test:ganache-cli",
     "test:gas": "GAS_REPORTER=true npm test",
     "test:gas:ci": "npm run test:gas && npx codechecks",
-    "lint": "solium --dir ./contracts",
-    "coverage": "SOLIDITY_COVERAGE=true npm run ganache-cli:test",
-    "ganache-cli:test": "scripts/ganache-cli.sh",
+    "test:ganache-cli": "./node_modules/@aragon/contract-helpers-test/scripts/ganache-cli.sh",
+    "coverage": "SOLIDITY_COVERAGE=true npm run test:ganache-cli",
     "abi:extract": "truffle-extract --output abi/ --keys abi",
     "bytecode:extract": "truffle-bytecode extract -o bytecode",
     "bytecode:extract:new": "truffle-bytecode extract -o bytecode_new",
@@ -29,17 +29,19 @@
   },
   "dependencies": {},
   "devDependencies": {
-    "@aragon/truffle-config-v4": "^1.0.1",
+    "@aragon/contract-helpers-test": "^0.1.0-rc.2",
+    "@aragon/truffle-config-v5": "^1.0.1",
     "@codechecks/client": "^0.1.5",
+    "chai": "^4.2.0",
     "coveralls": "^3.0.9",
     "eth-ens-namehash": "^2.0.8",
     "eth-gas-reporter": "^0.2.14",
     "ethereumjs-abi": "^0.6.5",
     "ganache-cli": "^6.9.0",
     "mocha-lcov-reporter": "^1.3.0",
-    "solidity-coverage": "0.6.2",
+    "solidity-coverage": "0.7.5",
     "solium": "^1.2.3",
-    "truffle": "4.1.14",
+    "truffle": "^5.0.34",
     "truffle-bytecode-manager": "^1.1.1",
     "truffle-extract": "^1.2.1",
     "web3-eth-abi": "1.2.5",

diff --git a/scripts/ganache-cli.sh b/scripts/ganache-cli.sh
diff --git a/test/contracts/acl/acl_params.js b/test/contracts/acl/acl_params.js
@@ -1,4 +1,4 @@
-const { assertRevert } = require('../../helpers/assertThrow')
+const { assertRevert } = require('@aragon/contract-helpers-test/src/asserts')
 const { skipSuiteCoverage } = require('../../helpers/coverage')
 const { permissionParamEqOracle } = require('../../helpers/permissionParams')
 
@@ -28,9 +28,9 @@ contract('ACL params', ([permissionsRoot, specificEntity, noPermission, mockAppA
   })
 
   beforeEach(async () => {
-    kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address)
+    kernel = await Kernel.at((await KernelProxy.new(kernelBase.address)).address)
     await kernel.initialize(aclBase.address, permissionsRoot)
-    acl = ACL.at(await kernel.acl())
+    acl = await ACL.at(await kernel.acl())
     await acl.createPermission(permissionsRoot, mockAppAddress, MOCK_APP_ROLE, permissionsRoot)
   })
 
@@ -142,8 +142,8 @@ contract('ACL params', ([permissionsRoot, specificEntity, noPermission, mockAppA
             it('ACL disallows and uses all gas when given large amount of gas', async () => {
               assert.isFalse(await acl.hasPermission(ANY_ADDR, mockAppAddress, MOCK_APP_ROLE, { gas: MAX_GAS_AVAILABLE }))
 
-              const hasPermissionTxHash = await acl.hasPermission.sendTransaction(ANY_ADDR, mockAppAddress, MOCK_APP_ROLE, { gas: MAX_GAS_AVAILABLE })
-              const hasPermissionGasConsumed = web3.eth.getTransactionReceipt(hasPermissionTxHash).gasUsed
+              const hasPermissionReceipt = await acl.hasPermission.sendTransaction(ANY_ADDR, mockAppAddress, MOCK_APP_ROLE, { gas: MAX_GAS_AVAILABLE })
+              const hasPermissionGasConsumed = hasPermissionReceipt.receipt.gasUsed
               // Surprisingly, the actual gas used is quite a lot lower than expected, but it is
               // unclear if this is a ganache issue or if there are gas refunds we're not taking
               // into account
@@ -153,8 +153,8 @@ contract('ACL params', ([permissionsRoot, specificEntity, noPermission, mockAppA
             it('ACL disallows and uses all gas when given medium amount of gas', async () => {
               assert.isFalse(await acl.hasPermission(ANY_ADDR, mockAppAddress, MOCK_APP_ROLE, { gas: MEDIUM_GAS }))
 
-              const hasPermissionTxHash = await acl.hasPermission.sendTransaction(ANY_ADDR, mockAppAddress, MOCK_APP_ROLE, { gas: MEDIUM_GAS })
-              const hasPermissionGasConsumed = web3.eth.getTransactionReceipt(hasPermissionTxHash).gasUsed
+              const hasPermissionReceipt = await acl.hasPermission.sendTransaction(ANY_ADDR, mockAppAddress, MOCK_APP_ROLE, { gas: MEDIUM_GAS })
+              const hasPermissionGasConsumed = hasPermissionReceipt.receipt.gasUsed
               assert.closeTo(hasPermissionGasConsumed, getExpectedGas(MEDIUM_GAS), 10000)
             })
 
@@ -179,8 +179,8 @@ contract('ACL params', ([permissionsRoot, specificEntity, noPermission, mockAppA
             it('ACL disallows and uses all gas when given large amount of gas', async () => {
               assert.isFalse(await acl.hasPermission(specificEntity, mockAppAddress, MOCK_APP_ROLE, { gas: MAX_GAS_AVAILABLE }))
 
-              const hasPermissionTxHash = await acl.hasPermission.sendTransaction(specificEntity, mockAppAddress, MOCK_APP_ROLE, { gas: MAX_GAS_AVAILABLE })
-              const hasPermissionGasConsumed = web3.eth.getTransactionReceipt(hasPermissionTxHash).gasUsed
+              const hasPermissionReceipt = await acl.hasPermission.sendTransaction(specificEntity, mockAppAddress, MOCK_APP_ROLE, { gas: MAX_GAS_AVAILABLE })
+              const hasPermissionGasConsumed = hasPermissionReceipt.receipt.gasUsed
               // Surprisingly, the actual gas used is quite a lot lower than expected, but it is
               // unclear if this is a ganache issue or if there are gas refunds we're not taking
               // into account
@@ -190,8 +190,8 @@ contract('ACL params', ([permissionsRoot, specificEntity, noPermission, mockAppA
             it('ACL disallows and uses all gas when given medium amount of gas', async () => {
               assert.isFalse(await acl.hasPermission(specificEntity, mockAppAddress, MOCK_APP_ROLE, { gas: MEDIUM_GAS }))
 
-              const hasPermissionTxHash = await acl.hasPermission.sendTransaction(specificEntity, mockAppAddress, MOCK_APP_ROLE, { gas: MEDIUM_GAS })
-              const hasPermissionGasConsumed = web3.eth.getTransactionReceipt(hasPermissionTxHash).gasUsed
+              const hasPermissionReceipt = await acl.hasPermission.sendTransaction(specificEntity, mockAppAddress, MOCK_APP_ROLE, { gas: MEDIUM_GAS })
+              const hasPermissionGasConsumed = hasPermissionReceipt.receipt.gasUsed
               assert.closeTo(hasPermissionGasConsumed, getExpectedGas(MEDIUM_GAS), 10000)
             })
 

diff --git a/test/contracts/apps/app_acl.js b/test/contracts/apps/app_acl.js
@@ -1,6 +1,6 @@
 const { hash } = require('eth-ens-namehash')
-const { onlyIf } = require('../../helpers/onlyIf')
-const { assertRevert } = require('../../helpers/assertThrow')
+const { assertRevert } = require('@aragon/contract-helpers-test/src/asserts')
+const { bn, onlyIf } = require('@aragon/contract-helpers-test')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')
@@ -12,8 +12,8 @@ const AppProxyPinned = artifacts.require('AppProxyPinned')
 const AppStub = artifacts.require('AppStub')
 const UnsafeAppStub = artifacts.require('UnsafeAppStub')
 
-const APP_ID = hash('stub.aragonpm.test')
 const EMPTY_BYTES = '0x'
+const APP_ID = hash('stub.aragonpm.test')
 
 contract('App ACL', accounts => {
   let aclBase, kernelBase, acl, kernel
@@ -35,9 +35,9 @@ contract('App ACL', accounts => {
   })
 
   beforeEach(async () => {
-    kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address)
+    kernel = await Kernel.at((await KernelProxy.new(kernelBase.address)).address)
     await kernel.initialize(aclBase.address, permissionsRoot)
-    acl = ACL.at(await kernel.acl())
+    acl = await ACL.at(await kernel.acl())
 
     // Set up app management permissions
     const APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
@@ -71,7 +71,7 @@ contract('App ACL', accounts => {
           } else if (appType === 'AppProxyPinned') {
             appProxy = await AppProxyPinned.new(kernel.address, APP_ID, EMPTY_BYTES)
           }
-          app = AppStub.at(appProxy.address)
+          app = await AppStub.at(appProxy.address)
         }
 
         await app.initialize()
@@ -102,7 +102,7 @@ contract('App ACL', accounts => {
         it('fails if using app proxy without reference in kernel', async () => {
           const unknownId = hash('unknown.aragonpm.test')
           const appProxy = await AppProxyUpgradeable.new(kernel.address, unknownId, EMPTY_BYTES)
-          const app = AppStub.at(appProxy.address)
+          const app = await AppStub.at(appProxy.address)
 
           await assertRevert(app.setValue(10))
         })
@@ -118,7 +118,7 @@ contract('App ACL', accounts => {
           const argId = '0x00' // arg 0
           const op = '03'    // greater than
           const value = `00000000000000000000000000000000000000000000000000000000000${paramValue}` // 5
-          const param = new web3.BigNumber(`${argId}${op}${value}`)
+          const param = bn(`${argId}${op}${value}`)
 
           await acl.grantPermissionP(paramsGrantee, app.address, APP_ROLE, [param], { from: permissionsRoot })
         })

diff --git a/test/contracts/apps/app_base_lifecycle.js b/test/contracts/apps/app_base_lifecycle.js
@@ -1,7 +1,6 @@
-const { hash } = require('eth-ens-namehash')
-const { soliditySha3 } = require('web3-utils')
-const { getBlockNumber } = require('../../helpers/web3')
-const { assertRevert } = require('../../helpers/assertThrow')
+const { sha3 } = require('web3-utils')
+const { assertRevert } = require('@aragon/contract-helpers-test/src/asserts')
+const { ZERO_ADDRESS } = require('@aragon/contract-helpers-test')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')
@@ -11,8 +10,7 @@ const AragonApp = artifacts.require('AragonApp')
 // Mocks
 const UnsafeAragonAppMock = artifacts.require('UnsafeAragonAppMock')
 
-const FAKE_ROLE = soliditySha3('FAKE_ROLE')
-const ZERO_ADDR = '0x0000000000000000000000000000000000000000'
+const FAKE_ROLE = sha3('FAKE_ROLE')
 
 contract('App base lifecycle', ([permissionsRoot]) => {
   let aclBase, kernelBase
@@ -80,15 +78,15 @@ contract('App base lifecycle', ([permissionsRoot]) => {
       })
 
       it('has correct initialization block', async () => {
-        assert.equal(await app.getInitializationBlock(), await getBlockNumber(), 'initialization block should be correct')
+        assert.equal(await app.getInitializationBlock(), await web3.eth.getBlockNumber(), 'initialization block should be correct')
       })
 
       it('throws on reinitialization', async () => {
         await assertRevert(app.initialize())
       })
 
       it('should still not be usable without a kernel', async () => {
-        assert.equal(await app.getKernel(), ZERO_ADDR, 'app should still be missing kernel reference')
+        assert.equal(await app.getKernel(), ZERO_ADDRESS, 'app should still be missing kernel reference')
 
         assert.isFalse(await app.canPerform(permissionsRoot, FAKE_ROLE, []))
       })
@@ -98,9 +96,9 @@ contract('App base lifecycle', ([permissionsRoot]) => {
 
         beforeEach(async () => {
           const kernelProxy = await KernelProxy.new(kernelBase.address)
-          kernel = Kernel.at(kernelProxy.address)
+          kernel = await Kernel.at(kernelProxy.address)
           await kernel.initialize(aclBase.address, permissionsRoot)
-          acl = ACL.at(await kernel.acl())
+          acl = await ACL.at(await kernel.acl())
 
           await app.setKernelOnMock(kernel.address)
         })

diff --git a/test/contracts/apps/app_funds.js b/test/contracts/apps/app_funds.js
@@ -1,7 +1,6 @@
 const { hash } = require('eth-ens-namehash')
-const { onlyIf } = require('../../helpers/onlyIf')
-const { assertRevert } = require('../../helpers/assertThrow')
-const { getBalance } = require('../../helpers/web3')
+const { bn, onlyIf } = require('@aragon/contract-helpers-test')
+const { assertRevert, assertBn } = require('@aragon/contract-helpers-test/src/asserts')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')
@@ -60,15 +59,15 @@ contract('App funds', ([permissionsRoot]) => {
           })
 
           beforeEach(async () => {
-            const kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address)
+            const kernel = await Kernel.at((await KernelProxy.new(kernelBase.address)).address)
             await kernel.initialize(aclBase.address, permissionsRoot)
 
             if (appType === 'App') {
               // Use the unsafe version to use directly without a proxy
               app = await unsafeAppBaseType.new(kernel.address)
             } else {
               // Install app
-              const acl = ACL.at(await kernel.acl())
+              const acl = await ACL.at(await kernel.acl())
               const APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
               await acl.createPermission(permissionsRoot, kernel.address, APP_MANAGER_ROLE, permissionsRoot)
               await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address)
@@ -80,7 +79,7 @@ contract('App funds', ([permissionsRoot]) => {
                 appProxy = await AppProxyPinned.new(kernel.address, APP_ID, EMPTY_BYTES)
               }
 
-              app = appBaseType.at(appProxy.address)
+              app = await appBaseType.at(appProxy.address)
             }
 
             await app.initialize()
@@ -99,14 +98,14 @@ contract('App funds', ([permissionsRoot]) => {
             })
 
             it('can receive ETH after being set to depositable', async () => {
-              const amount = 1
-              const initialBalance = await getBalance(app.address)
+              const amount = bn(1)
+              const initialBalance = bn(await web3.eth.getBalance(app.address))
 
               await app.enableDeposits()
               assert.isTrue(await app.isDepositable(), 'should be depositable')
 
               await app.sendTransaction({ value: 1, gas: SEND_ETH_GAS })
-              assert.equal((await getBalance(app.address)).valueOf(), initialBalance.plus(amount))
+              assertBn(bn(await web3.eth.getBalance(app.address)), initialBalance.add(amount))
             })
           })
         })

diff --git a/test/contracts/apps/app_interface.js b/test/contracts/apps/app_interface.js
@@ -1,5 +1,5 @@
-const { getEventArgument } = require('../../helpers/events')
-const { getNewProxyAddress } = require('../../helpers/events')
+const { getEventArgument } = require('@aragon/contract-helpers-test')
+const { getInstalledApp } = require('@aragon/contract-helpers-test/src/aragon-os')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')
@@ -8,7 +8,7 @@ const AragonApp = artifacts.require('AragonAppMock')
 const ERC165 = artifacts.require('ERC165Mock')
 const EVMScriptRegistryFactory = artifacts.require('EVMScriptRegistryFactory')
 
-contract('AragonApp', ([_, owner, agreement, anotherAgreement, someone]) => {
+contract('AragonApp', ([_, owner]) => {
   let aragonApp
 
   const ARAGON_APP_INTERFACE = '0x54053e6c'
@@ -21,15 +21,15 @@ contract('AragonApp', ([_, owner, agreement, anotherAgreement, someone]) => {
     const daoFact = await DAOFactory.new(kernelBase.address, aclBase.address, registryFactory.address)
 
     const receiptDao = await daoFact.newDAO(owner)
-    dao = await Kernel.at(getEventArgument(receiptDao, 'DeployDAO', 'dao'))
-    acl = await ACL.at(await dao.acl())
+    const dao = await Kernel.at(getEventArgument(receiptDao, 'DeployDAO', 'dao'))
+    const acl = await ACL.at(await dao.acl())
     const aragonAppBase = await AragonApp.new()
 
     const APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
     await acl.createPermission(owner, dao.address, APP_MANAGER_ROLE, owner, { from: owner })
-    const initializeData = aragonAppBase.contract.initialize.getData()
+    const initializeData = aragonAppBase.contract.methods.initialize().encodeABI()
     const receiptInstance = await dao.newAppInstance('0x1234', aragonAppBase.address, initializeData, false, { from: owner })
-    aragonApp = await AragonApp.at(getNewProxyAddress(receiptInstance))
+    aragonApp = await AragonApp.at(getInstalledApp(receiptInstance))
   })
 
   describe('supportsInterface', () => {