chore: upgrade solidity-coverage to v0.6+ (#541)

.solcover.js

@@ -3,7 +3,6 @@ const skipFiles = [
   'test',
   'acl/ACLSyntaxSugar.sol',
   'common/DepositableStorage.sol', // Used in tests that send ETH
-  'common/SafeERC20.sol', // solidity-coverage fails on assembly if (https://github.com/sc-forks/solidity-coverage/issues/287)
   'common/UnstructuredStorage.sol' // Used in tests that send ETH
 ]
 

package.json

@@ -39,7 +39,7 @@
     "ethereumjs-abi": "^0.6.5",
     "ganache-cli": "^6.4.2",
     "mocha-lcov-reporter": "^1.3.0",
-    "solidity-coverage": "0.5.8",
+    "solidity-coverage": "^0.6.2",
     "solium": "^1.2.3",
     "truffle": "4.1.14",
     "truffle-bytecode-manager": "^1.1.1",

test/contracts/apps/app_funds.js

@@ -1,7 +1,7 @@
 const { hash } = require('eth-ens-namehash')
 const { onlyIf } = require('../../helpers/onlyIf')
-const { getBalance } = require('../../helpers/web3')
 const { assertRevert } = require('../../helpers/assertThrow')
+const { getBalance } = require('../../helpers/web3')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')

test/contracts/apps/recovery_to_vault.js

@@ -1,9 +1,8 @@
 const { hash } = require('eth-ens-namehash')
-const { getBalance } = require('../../helpers/web3')
-const { skipCoverage } = require('../../helpers/coverage')
+const { assertAmountOfEvents, assertEvent } = require('../../helpers/assertEvent')(web3)
 const { assertRevert } = require('../../helpers/assertThrow')
 const { getNewProxyAddress } = require('../../helpers/events')
-const { assertAmountOfEvents, assertEvent } = require('../../helpers/assertEvent')(web3)
+const { getBalance } = require('../../helpers/web3')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')
@@ -160,15 +159,6 @@ contract('Recovery to vault', ([permissionsRoot]) => {
 
       // Test both the Vault itself and when it's behind a proxy to make sure their recovery behaviours are the same
       for (const vaultType of ['Vault', 'VaultProxy']) {
-        const skipCoverageIfVaultProxy = test => {
-          // The VaultMock isn't instrumented during coverage, but the AppProxy is, and so
-          // transferring to the fallback fails when we're testing with the proxy.
-          // Native transfers (either .send() or .transfer()) fail under coverage because they're
-          // limited to 2.3k gas, and the injected instrumentation from coverage makes these
-          // operations cost more than that limit.
-          return vaultType === 'VaultProxy' ? skipCoverage(test) : test
-        }
-
         context(`> ${vaultType}`, () => {
           let target, vault
 
@@ -218,9 +208,9 @@ contract('Recovery to vault', ([permissionsRoot]) => {
               await target.enableDeposits()
             })
 
-            it('does not recover ETH', skipCoverageIfVaultProxy(async () =>
+            it('does not recover ETH', async () =>
               await recoverEth({ target, vault, shouldFail: true })
-            ))
+            )
 
             it('does not recover tokens', async () =>
               await recoverTokens({
@@ -251,9 +241,9 @@ contract('Recovery to vault', ([permissionsRoot]) => {
               await assertRevert(target.sendTransaction({ value: 1, data: '0x01', gas: SEND_ETH_GAS }))
             })
 
-            it('recovers ETH', skipCoverageIfVaultProxy(async () =>
+            it('recovers ETH', async () =>
               await recoverEth({ target, vault })
-            ))
+            )
 
             for (const { title, tokenContract } of tokenTestGroups) {
               it(`recovers ${title}`, async () => {
@@ -289,10 +279,10 @@ contract('Recovery to vault', ([permissionsRoot]) => {
               target = app
             })
 
-            it('does not allow recovering ETH', skipCoverageIfVaultProxy(async () =>
+            it('does not allow recovering ETH', async () =>
               // Conditional stub doesnt allow eth recoveries
               await recoverEth({ target, vault, shouldFail: true })
-            ))
+            )
 
             for (const { title, tokenContract } of tokenTestGroups) {
               it(`allows recovers ${title}`, async () => {
@@ -344,8 +334,8 @@ contract('Recovery to vault', ([permissionsRoot]) => {
       await kernel.setRecoveryVaultAppId(vaultId)
     })
 
-    it('recovers ETH from the kernel', skipCoverage(async () => {
+    it('recovers ETH from the kernel', async () => {
       await recoverEth({ target: kernel, vault })
-    }))
+    })
   })
 })

test/contracts/kernel/kernel_funds.js

@@ -1,6 +1,6 @@
+const { assertRevert } = require('../../helpers/assertThrow')
 const { onlyIf } = require('../../helpers/onlyIf')
 const { getBalance } = require('../../helpers/web3')
-const { assertRevert } = require('../../helpers/assertThrow')
 
 const ACL = artifacts.require('ACL')
 const Kernel = artifacts.require('Kernel')

test/helpers/assertThrow.js

@@ -31,7 +31,7 @@ module.exports = {
       error.reason = error.message.replace(errorPrefix, '').trim()
     }
 
-    if (process.env.SOLIDITY_COVERAGE !== 'true' && reason) {
+    if (reason) {
       assert.equal(error.reason, reason, `Expected revert reason "${reason}" but failed with "${error.reason || 'no reason'}" instead.`)
     }
   },