Fix CI (aragon#1172)

* Upgrade aragonOS dependency For Vault, Agent and Finance. To fix Istanbul related issues. See https://github.com/aragon/aragonOS/releases/tag/v4.3.0 for more context. * payroll: fix gas values in tests * agent: fix tests `isValidSignature` was calling the `bytes` version due to overloading issue with Truffle. * Fix CI, address PR aragon#1172 comments
ConsiderItDone · Jun 17, 2020 · 593549a · 593549a
1 parent 9a1a45d
commit 593549a
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 30 deletions.
diff --git a/apps/agent/contracts/test/mocks/AgentMock.sol b/apps/agent/contracts/test/mocks/AgentMock.sol
@@ -0,0 +1,12 @@
+pragma solidity ^0.4.24;
+
+import "../../Agent.sol";
+
+
+contract AgentMock is Agent {
+    // truffle doesn’t work well with Solidity function overloading, so we create this wrapper to make sure
+    // the function in Agent is called, instead of the one in ERC1271Bytes
+    function isValidSignatureBytes32(bytes32 _hash, bytes memory _signature) public view returns (bytes4) {
+        return isValidSignature(_hash, _signature);
+    }
+}
diff --git a/apps/agent/package.json b/apps/agent/package.json
@@ -53,6 +53,6 @@
   },
   "dependencies": {
     "@aragon/apps-vault": "4.1.0",
-    "@aragon/os": "4.2.0"
+    "@aragon/os": "4.4.0"
   }
 }
diff --git a/apps/agent/test/agent.js b/apps/agent/test/agent.js
@@ -1,5 +1,5 @@
 const runSharedAgentTests = require('./agent_shared.js')
 
 contract('Agent', (accounts) => {
-  runSharedAgentTests('Agent', { accounts, artifacts, web3 })
+  runSharedAgentTests('AgentMock', { accounts, artifacts, web3 })
 })
diff --git a/apps/agent/test/agent_shared.js b/apps/agent/test/agent_shared.js
@@ -770,18 +770,18 @@ module.exports = (
         })
 
         it('isValidSignature returns false if there is not designated signer and hash isn\'t presigned', async () => {
-          assertIsValidSignature(false, await agent.isValidSignature(HASH, NO_SIG))
+          assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, NO_SIG))
         })
 
         it('presigns a hash', async () => {
           await agent.presignHash(HASH, { from: presigner })
 
-          assertIsValidSignature(true, await agent.isValidSignature(HASH, NO_SIG))
+          assertIsValidSignature(true, await agent.isValidSignatureBytes32(HASH, NO_SIG))
         })
 
         it('fails to presign a hash if not authorized', async () => {
           await assertRevert(agent.presignHash(HASH, { from: nobody }), errors.APP_AUTH_FAILED)
-          assertIsValidSignature(false, await agent.isValidSignature(HASH, NO_SIG))
+          assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, NO_SIG))
         })
 
         context('> Designated signer', () => {
@@ -876,27 +876,27 @@ module.exports = (
 
               it('isValidSignature returns true to a valid signature', async () => {
                 const signature = await sign(HASH, signerOrKey)
-                assertIsValidSignature(true, await agent.isValidSignature(HASH, signature))
+                assertIsValidSignature(true, await agent.isValidSignatureBytes32(HASH, signature))
               })
 
               it('isValidSignature returns true to a valid signature with legacy version', async () => {
                 const legacyVersionSignature = await sign(HASH, signerOrKey, true)
-                assertIsValidSignature(true, await agent.isValidSignature(HASH, legacyVersionSignature))
+                assertIsValidSignature(true, await agent.isValidSignatureBytes32(HASH, legacyVersionSignature))
               })
 
               it('isValidSignature returns false to an invalid signature', async () => {
                 const badSignature = (await sign(HASH, signerOrKey)).slice(0, -2) // drop last byte
-                assertIsValidSignature(false, await agent.isValidSignature(HASH, badSignature))
+                assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, badSignature))
               })
 
               it('isValidSignature returns false to a signature with an invalid v', async () => {
                 const invalidVersionSignature = await sign(HASH, signerOrKey, false, true)
-                assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidVersionSignature))
+                assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, invalidVersionSignature))
               })
 
               it('isValidSignature returns false to an unauthorized signer', async () => {
                 const otherSignature = await sign(HASH, notSignerOrKey)
-                assertIsValidSignature(false, await agent.isValidSignature(HASH, otherSignature))
+                assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, otherSignature))
               })
             })
           }
@@ -916,7 +916,7 @@ module.exports = (
               // false - doesn't modify state on checking sig
               await setDesignatedSignerContract(true, true, false, false)
 
-              assertIsValidSignature(true, await agent.isValidSignature(HASH, ERC1271_SIG))
+              assertIsValidSignature(true, await agent.isValidSignatureBytes32(HASH, ERC1271_SIG))
             })
 
             it('isValidSignature returns false if designated signer returns false', async () => {
@@ -927,7 +927,7 @@ module.exports = (
               await setDesignatedSignerContract(true, false, false, false)
 
               // Signature fails check
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, ERC1271_SIG))
             })
 
             it('isValidSignature returns true even if the designated signer doesnt support the interface', async () => {
@@ -937,7 +937,7 @@ module.exports = (
               // false - doesn't modify state on checking sig
               await setDesignatedSignerContract(false, true, false, false)
 
-              assertIsValidSignature(true, await agent.isValidSignature(HASH, ERC1271_SIG))
+              assertIsValidSignature(true, await agent.isValidSignatureBytes32(HASH, ERC1271_SIG))
             })
 
             it('isValidSignature returns false if designated signer reverts', async () => {
@@ -948,7 +948,7 @@ module.exports = (
               await setDesignatedSignerContract(true, true, true, false)
 
               // Reverts on checking
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, ERC1271_SIG))
             })
 
             it('isValidSignature returns false if designated signer attempts to modify state', async () => {
@@ -959,7 +959,7 @@ module.exports = (
               await setDesignatedSignerContract(true, true, false, true)
 
               // Checking costs too much gas
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, ERC1271_SIG))
             })
           })
 
@@ -972,26 +972,26 @@ module.exports = (
 
             it('isValidSignature returns false to an empty signature', async () => {
               const emptySig = '0x'
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, emptySig))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, emptySig))
             })
 
             it('isValidSignature returns false to an invalid mode signature', async () => {
               const invalidSignature = SIGNATURE_MODES.Invalid
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidSignature))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, invalidSignature))
             })
 
             it('isValidSignature returns false to an unspecified mode signature', async () => {
               const unspecifiedSignature = SIGNATURE_MODES.NMode
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, unspecifiedSignature))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, unspecifiedSignature))
             })
 
             it('isValidSignature returns true to an invalid signature iff the hash was presigned', async () => {
               const invalidSignature = SIGNATURE_MODES.Invalid
-              assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidSignature))
+              assertIsValidSignature(false, await agent.isValidSignatureBytes32(HASH, invalidSignature))
 
               // Now presign it
               await agent.presignHash(HASH, { from: presigner })
-              assertIsValidSignature(true, await agent.isValidSignature(HASH, invalidSignature))
+              assertIsValidSignature(true, await agent.isValidSignatureBytes32(HASH, invalidSignature))
             })
           })
 

diff --git a/apps/finance/package.json b/apps/finance/package.json
@@ -53,6 +53,6 @@
   },
   "dependencies": {
     "@aragon/apps-vault": "^4.0.0",
-    "@aragon/os": "4.2.0"
+    "@aragon/os": "4.4.0"
   }
 }
diff --git a/apps/vault/package.json b/apps/vault/package.json
@@ -49,6 +49,6 @@
     "truffle-extract": "^1.2.1"
   },
   "dependencies": {
-    "@aragon/os": "4.2.0"
+    "@aragon/os": "4.4.0"
   }
 }
diff --git a/future-apps/payroll/test/contracts/Payroll_gas_costs.test.js b/future-apps/payroll/test/contracts/Payroll_gas_costs.test.js
@@ -35,27 +35,30 @@ contract('Payroll gas costs', ([owner, employee, anotherEmployee]) => {
     })
 
     context('when there is only one allowed token', function () {
-      it('expends ~337k gas for a single allowed token', async () => {
+      const gas = 382
+      it(`expends ~${gas}k gas for a single allowed token`, async () => {
         await payroll.determineAllocation([DAI.address], [100], { from: employee })
 
         const { receipt: { cumulativeGasUsed } } = await payroll.payday(PAYMENT_TYPES.PAYROLL, 0, [], { from: employee })
 
         console.log('cumulativeGasUsed:', cumulativeGasUsed)
-        assert.isAtMost(cumulativeGasUsed, 337000, 'payout gas cost for a single allowed token should be ~338k')
+        assert.isAtMost(cumulativeGasUsed, gas * 1000, `payout gas cost for a single allowed token should be ~${gas}k`)
       })
     })
 
     context('when there are two allowed token', function () {
-      it('expends ~49k gas in setting a allocation tokens per token', async () => {
+      const gas_set_allocation = 49
+      it(`expends ~${gas_set_allocation}k gas in setting a allocation tokens per token`, async () => {
         const { receipt: { cumulativeGasUsed: oneTokenAllocationGasUsed } } = await payroll.determineAllocation([DAI.address], [100], { from: employee })
         const { receipt: { cumulativeGasUsed: twoTokensAllocationGasUsed } } = await payroll.determineAllocation([DAI.address, ANT.address], [60, 40], { from: anotherEmployee })
 
         const gasPerAllocationToken = twoTokensAllocationGasUsed - oneTokenAllocationGasUsed
         console.log('gasPerAllocationToken:', gasPerAllocationToken)
-        assert.isAtMost(gasPerAllocationToken, 49000, 'gas cost increment of setting allocation token sets should be ~49k per token')
+        assert.isAtMost(gasPerAllocationToken, gas_set_allocation * 1000, `gas cost increment of setting allocation token sets should be ~${gas_set_allocation}k per token`)
       })
 
-      it('expends ~30k gas in overwriting an allocation set per token', async () => {
+      const gas_overwrite_allocation = 30
+      it(`expends ~${gas_overwrite_allocation}k gas in overwriting an allocation set per token`, async () => {
         await payroll.determineAllocation([DAI.address], [100], { from: employee })
         const { receipt: { cumulativeGasUsed: oneTokenAllocationOverwriteGasUsed } } = await payroll.determineAllocation([ANT.address], [100], { from: employee })
 
@@ -64,10 +67,11 @@ contract('Payroll gas costs', ([owner, employee, anotherEmployee]) => {
 
         const gasPerAllocationToken = twoTokensAllocationOverwriteGasUsed - oneTokenAllocationOverwriteGasUsed
         console.log('gasPerAllocationToken:', gasPerAllocationToken)
-        assert.isAtMost(gasPerAllocationToken, 30000, 'gas cost increment of overwriting allocation token sets should be ~30k per token')
+        assert.isAtMost(gasPerAllocationToken, gas_overwrite_allocation * 1000, `gas cost increment of overwriting allocation token sets should be ~${gas_overwrite_allocation}k per token`)
       })
 
-      it('expends ~295k gas in payday per allowed token', async () => {
+      const gas_payday = 323
+      it(`expends ~${gas_payday}k gas in payday per allowed token`, async () => {
         await payroll.determineAllocation([DAI.address], [100], { from: employee })
         const { receipt: { cumulativeGasUsed: employeePayoutGasUsed } } = await payroll.payday(PAYMENT_TYPES.PAYROLL, 0, [], { from: employee })
 
@@ -76,7 +80,7 @@ contract('Payroll gas costs', ([owner, employee, anotherEmployee]) => {
 
         const gasPerAllowedToken = anotherEmployeePayoutGasUsed - employeePayoutGasUsed
         console.log('gasPerAllowedToken:', gasPerAllowedToken)
-        assert.isAtMost(gasPerAllowedToken, 295000, 'payroll payday gas cost increment per allowed token should be ~295k')
+        assert.isAtMost(gasPerAllowedToken, gas_payday * 1000, `payroll payday gas cost increment per allowed token should be ~${gas_payday}k`)
       })
     })
   })