Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI #1172

Merged
merged 4 commits into from
Jun 17, 2020
Merged

Fix CI #1172

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions apps/agent/contracts/test/mocks/AgentMock.sol
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we leave a comment on why this is necessary?

return isValidSignature(_hash, _signature);
}
}
2 changes: 1 addition & 1 deletion apps/agent/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@
},
"dependencies": {
"@aragon/apps-vault": "4.1.0",
"@aragon/os": "4.2.0"
"@aragon/os": "4.4.0"
}
}
2 changes: 1 addition & 1 deletion apps/agent/test/agent.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const runSharedAgentTests = require('./agent_shared.js')

contract('Agent', (accounts) => {
runSharedAgentTests('Agent', { accounts, artifacts, web3 })
runSharedAgentTests('AgentMock', { accounts, artifacts, web3 })
})
36 changes: 18 additions & 18 deletions apps/agent/test/agent_shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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))
})
})
}
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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))
})
})

Expand All @@ -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))
})
})

Expand Down
2 changes: 1 addition & 1 deletion apps/finance/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@
},
"dependencies": {
"@aragon/apps-vault": "^4.0.0",
"@aragon/os": "4.2.0"
"@aragon/os": "4.4.0"
}
}
2 changes: 1 addition & 1 deletion apps/vault/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@
"truffle-extract": "^1.2.1"
},
"dependencies": {
"@aragon/os": "4.2.0"
"@aragon/os": "4.4.0"
}
}
20 changes: 12 additions & 8 deletions future-apps/payroll/test/contracts/Payroll_gas_costs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })

Expand All @@ -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 })

Expand All @@ -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`)
})
})
})
Expand Down