From 635dd9633f9dac5f55b23ba94c0d32f032c82e18 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Tue, 23 Apr 2019 13:05:22 +0200 Subject: [PATCH 01/10] Testing improvements (#513) * test: add tests for non-existent executors * test: add explicit lifecycle tests for duplicate initializations * test: fail solidity tests if they made no assertions --- contracts/test/TestDelegateProxy.sol | 3 +++ test/evm_script.js | 27 +++++++++++++++++++++++---- test/helpers/runSolidityTest.js | 19 ++++++++++++------- test/lifecycle.js | 13 +++++++++++++ 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/contracts/test/TestDelegateProxy.sol b/contracts/test/TestDelegateProxy.sol index 3fdc83ab9..adce13545 100644 --- a/contracts/test/TestDelegateProxy.sol +++ b/contracts/test/TestDelegateProxy.sol @@ -69,6 +69,9 @@ contract TestDelegateProxy is DelegateProxy { // keep as last test as it will kill this contract function testDieIfMinReturn0() public { + Assert.isTrue(true, ''); // Make at least one assertion to satisfy the runner + delegatedFwd(target, target.die.selector.toBytes()); + Assert.fail('should be dead'); } } diff --git a/test/evm_script.js b/test/evm_script.js index 19fb34fc3..28c802a9c 100644 --- a/test/evm_script.js +++ b/test/evm_script.js @@ -160,16 +160,35 @@ contract('EVM Script', accounts => { await assertRevert(evmScriptReg.enableScriptExecutor(installedExecutorId, { from: boss }), reverts.EVMREG_EXECUTOR_ENABLED) }) + it('fails to disable an already disabled executor', async () => { + await acl.createPermission(boss, evmScriptReg.address, REGISTRY_MANAGER_ROLE, boss, { from: boss }) + await evmScriptReg.disableScriptExecutor(installedExecutorId, { from: boss }) + + await assertRevert(evmScriptReg.disableScriptExecutor(installedExecutorId, { from: boss }), reverts.EVMREG_EXECUTOR_DISABLED) + }) + }) + + context('> Non-existing executor', () => { it('fails to enable a non-existent executor', async () => { await acl.createPermission(boss, evmScriptReg.address, REGISTRY_MANAGER_ROLE, boss, { from: boss }) - await assertRevert(evmScriptReg.enableScriptExecutor(installedExecutorId + 1, { from: boss }), reverts.EVMREG_INEXISTENT_EXECUTOR) + + // 0 is reserved + await assertRevert(evmScriptReg.enableScriptExecutor(0, { from: boss }), reverts.EVMREG_INEXISTENT_EXECUTOR) + + // No executors should be installed yet + assert.equal(await evmScriptReg.getScriptExecutor(createExecutorId(1)), ZERO_ADDR, 'No executors should be installed yet') + await assertRevert(evmScriptReg.enableScriptExecutor(1, { from: boss }), reverts.EVMREG_INEXISTENT_EXECUTOR) }) - it('fails to disable an already disabled executor', async () => { + it('fails to disable a non-existent executor', async () => { await acl.createPermission(boss, evmScriptReg.address, REGISTRY_MANAGER_ROLE, boss, { from: boss }) - await evmScriptReg.disableScriptExecutor(installedExecutorId, { from: boss }) - await assertRevert(evmScriptReg.disableScriptExecutor(installedExecutorId, { from: boss }), reverts.EVMREG_EXECUTOR_DISABLED) + assert.equal(await evmScriptReg.getScriptExecutor(createExecutorId(1)), ZERO_ADDR, 'No executors should be installed yet') + await assertRevert( + evmScriptReg.disableScriptExecutor(1, { from: boss }), + // On disable only an enable check is performed as it doubles as an existence check + reverts.EVMREG_EXECUTOR_DISABLED + ) }) }) }) diff --git a/test/helpers/runSolidityTest.js b/test/helpers/runSolidityTest.js index 1978769e1..adbb74197 100644 --- a/test/helpers/runSolidityTest.js +++ b/test/helpers/runSolidityTest.js @@ -27,7 +27,7 @@ const HOOKS_MAP = { afterAll: 'afterAll', } -const processResult = txReceipt => { +const processResult = (txReceipt, mustAssert) => { if (!txReceipt || !txReceipt.receipt) { return } @@ -37,6 +37,9 @@ const processResult = txReceipt => { throw new Error(log.args.message) } }) + if (mustAssert && !decodedLogs.length) { + throw new Error('No assertions made') + } } /* @@ -81,13 +84,15 @@ function runSolidityTest(c, mochaContext) { if (interface.type === 'function') { if (['beforeAll', 'beforeEach', 'afterEach', 'afterAll'].includes(interface.name)) { // Set up hooks - global[HOOKS_MAP[interface.name]](() => { - return deployed[interface.name]().then(processResult) - }) + global[HOOKS_MAP[interface.name]](() => + deployed[interface.name]() + .then(receipt => processResult(receipt, false)) + ) } else if (interface.name.startsWith('test')) { - it(interface.name, () => { - return deployed[interface.name]().then(processResult) - }) + it(interface.name, () => + deployed[interface.name]() + .then(receipt => processResult(receipt, true)) + ) } } }) diff --git a/test/lifecycle.js b/test/lifecycle.js index 25714746d..ce72ef45d 100644 --- a/test/lifecycle.js +++ b/test/lifecycle.js @@ -1,5 +1,6 @@ const { assertRevert } = require('./helpers/assertThrow') const { getBlockNumber } = require('./helpers/web3') +const reverts = require('./helpers/revertStrings') // Mocks const LifecycleMock = artifacts.require('LifecycleMock') @@ -35,6 +36,14 @@ contract('Lifecycle', accounts => { it('has correct initialization block', async () => { assert.equal(await lifecycle.getInitializationBlock(), await getBlockNumber(), 'initialization block should be correct') }) + + it('cannot be re-initialized', async () => { + assertRevert(lifecycle.initializeMock(), reverts.INIT_ALREADY_INITIALIZED) + }) + + it('cannot be petrified', async () => { + assertRevert(lifecycle.petrifyMock(), reverts.INIT_ALREADY_INITIALIZED) + }) }) context('> Petrified', () => { @@ -50,6 +59,10 @@ contract('Lifecycle', accounts => { assert.isTrue(await lifecycle.isPetrified(), 'should be petrified') }) + it('cannot be petrified again', async () => { + assertRevert(lifecycle.petrifyMock(), reverts.INIT_ALREADY_INITIALIZED) + }) + it('has initialization block in the future', async () => { const petrifiedBlock = await lifecycle.getInitializationBlock() const blockNumber = await getBlockNumber() From 7052952ee0521f242142b9f316d7e105ecd609ad Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Thu, 2 May 2019 15:01:12 -0300 Subject: [PATCH 02/10] Organize mocks and test files (#517) * contracts: organize mock contracts in sub dirs * tests: organize contract tests in sub dirs --- contracts/test/mocks/{ => apm}/APMNamehashMock.sol | 2 +- contracts/test/mocks/{ => apm}/UnsafeRepo.sol | 4 ++-- .../mocks/{ => apps}/AppProxyPinnedStorageMock.sol | 6 +++--- contracts/test/mocks/{ => apps}/AppStorageMock.sol | 2 +- contracts/test/mocks/{ => apps}/AppStub.sol | 6 +++--- .../mocks/{ => apps}/AppStubConditionalRecovery.sol | 4 ++-- contracts/test/mocks/{ => apps}/AppStubDepositable.sol | 6 +++--- .../test/mocks/{ => apps}/AppStubScriptRunner.sol | 2 +- .../test/mocks/{ => apps}/UnsafeAragonAppMock.sol | 4 ++-- contracts/test/mocks/{ => apps}/VaultMock.sol | 4 ++-- .../test/mocks/{ => common}/DepositableStorageMock.sol | 2 +- .../test/mocks/{ => common}/EtherTokenConstantMock.sol | 2 +- .../mocks/{ => common}/InitializableStorageMock.sol | 2 +- contracts/test/mocks/{ => common}/KeccakConstants.sol | 0 contracts/test/mocks/{ => common}/LifecycleMock.sol | 4 ++-- .../test/mocks/{ => common}/ReentrancyGuardMock.sol | 4 ++-- contracts/test/mocks/{ => common}/TimeHelpersMock.sol | 2 +- contracts/test/mocks/{ => common}/Uint256Mock.sol | 2 +- contracts/test/mocks/{ => ens}/ENSConstantsMock.sol | 2 +- .../mocks/{ => evmscript}/EVMScriptExecutorMock.sol | 2 +- .../{ => evmscript}/EVMScriptExecutorNoReturnMock.sol | 2 +- .../{ => evmscript}/EVMScriptExecutorRevertMock.sol | 2 +- .../{ => evmscript}/EVMScriptRegistryConstantsMock.sol | 2 +- .../test/mocks/{ => evmscript}/ExecutionTarget.sol | 0 .../mocks/{ => factory}/APMRegistryFactoryMock.sol | 10 +++++----- .../test/mocks/{ => kernel}/KernelConstantsMock.sol | 2 +- .../test/mocks/{ => kernel}/KernelDepositableMock.sol | 4 ++-- .../test/mocks/{ => kernel}/KernelOverloadMock.sol | 4 ++-- contracts/test/mocks/{ => kernel}/KernelSetAppMock.sol | 2 +- contracts/test/mocks/{ => kernel}/UpgradedKernel.sol | 2 +- contracts/test/mocks/{ => lib/math}/SafeMath64Mock.sol | 2 +- contracts/test/mocks/{ => lib/math}/SafeMath8Mock.sol | 2 +- contracts/test/mocks/{ => lib/misc}/ERCProxyMock.sol | 2 +- contracts/test/mocks/{ => lib/token}/SafeERC20Mock.sol | 4 ++-- contracts/test/mocks/{ => lib/token}/TokenMock.sol | 2 +- .../mocks/{ => lib/token}/TokenReturnFalseMock.sol | 0 .../mocks/{ => lib/token}/TokenReturnMissingMock.sol | 2 +- contracts/test/{ => tests}/TestACLInterpreter.sol | 6 +++--- contracts/test/{ => tests}/TestConversionHelpers.sol | 6 +++--- contracts/test/{ => tests}/TestDelegateProxy.sol | 8 ++++---- test/acl_interpreter.js | 3 --- test/contracts/acl/acl_interpreter.js | 3 +++ test/{ => contracts/apm}/apm_namehash.js | 0 test/{ => contracts/apm}/apm_registry.js | 2 +- test/{ => contracts/apm}/apm_repo.js | 2 +- test/{ => contracts/apps}/app_acl.js | 4 ++-- test/{ => contracts/apps}/app_base_lifecycle.js | 4 ++-- test/{ => contracts/apps}/app_funds.js | 6 +++--- test/{ => contracts/apps}/app_proxy.js | 6 +++--- test/{ => contracts/apps}/recovery_to_vault.js | 8 ++++---- test/contracts/common/conversion_helpers.js | 3 +++ test/contracts/common/delegate_proxy.js | 3 +++ test/{ => contracts/common}/keccak_constants.js | 0 test/{ => contracts/common}/lib_uint256_helpers.js | 2 +- test/{ => contracts/common}/lifecycle.js | 6 +++--- test/{ => contracts/common}/reentrancy_guard.js | 2 +- test/{ => contracts/common}/time_helpers.js | 0 test/{ => contracts/common}/unstructured_storage.js | 0 test/{ => contracts/ens}/ens_subdomains.js | 2 +- test/{ => contracts/evmscript}/evm_script.js | 8 ++++---- test/{ => contracts/factory}/evm_script_factory.js | 2 +- test/{ => contracts/kernel}/kernel_acl.js | 4 ++-- test/{ => contracts/kernel}/kernel_apps.js | 6 +++--- test/{ => contracts/kernel}/kernel_funds.js | 6 +++--- test/{ => contracts/kernel}/kernel_lifecycle.js | 6 +++--- test/{ => contracts/kernel}/kernel_upgrade.js | 4 ++-- test/{ => contracts/lib/math}/lib_safemath64.js | 2 +- test/{ => contracts/lib/math}/lib_safemath8.js | 2 +- test/{ => contracts/lib/token}/safe_erc20.js | 2 +- test/conversion_helpers.js | 3 --- test/delegate_proxy.js | 3 --- 71 files changed, 115 insertions(+), 115 deletions(-) rename contracts/test/mocks/{ => apm}/APMNamehashMock.sol (87%) rename contracts/test/mocks/{ => apm}/UnsafeRepo.sol (79%) rename contracts/test/mocks/{ => apps}/AppProxyPinnedStorageMock.sol (90%) rename contracts/test/mocks/{ => apps}/AppStorageMock.sol (91%) rename contracts/test/mocks/{ => apps}/AppStub.sol (88%) rename contracts/test/mocks/{ => apps}/AppStubConditionalRecovery.sol (80%) rename contracts/test/mocks/{ => apps}/AppStubDepositable.sol (78%) rename contracts/test/mocks/{ => apps}/AppStubScriptRunner.sol (98%) rename contracts/test/mocks/{ => apps}/UnsafeAragonAppMock.sol (79%) rename contracts/test/mocks/{ => apps}/VaultMock.sol (77%) rename contracts/test/mocks/{ => common}/DepositableStorageMock.sol (86%) rename contracts/test/mocks/{ => common}/EtherTokenConstantMock.sol (76%) rename contracts/test/mocks/{ => common}/InitializableStorageMock.sol (86%) rename contracts/test/mocks/{ => common}/KeccakConstants.sol (100%) rename contracts/test/mocks/{ => common}/LifecycleMock.sol (71%) rename contracts/test/mocks/{ => common}/ReentrancyGuardMock.sol (93%) rename contracts/test/mocks/{ => common}/TimeHelpersMock.sol (94%) rename contracts/test/mocks/{ => common}/Uint256Mock.sol (80%) rename contracts/test/mocks/{ => ens}/ENSConstantsMock.sol (93%) rename contracts/test/mocks/{ => evmscript}/EVMScriptExecutorMock.sol (89%) rename contracts/test/mocks/{ => evmscript}/EVMScriptExecutorNoReturnMock.sol (86%) rename contracts/test/mocks/{ => evmscript}/EVMScriptExecutorRevertMock.sol (87%) rename contracts/test/mocks/{ => evmscript}/EVMScriptRegistryConstantsMock.sol (80%) rename contracts/test/mocks/{ => evmscript}/ExecutionTarget.sol (100%) rename contracts/test/mocks/{ => factory}/APMRegistryFactoryMock.sol (93%) rename contracts/test/mocks/{ => kernel}/KernelConstantsMock.sol (95%) rename contracts/test/mocks/{ => kernel}/KernelDepositableMock.sol (79%) rename contracts/test/mocks/{ => kernel}/KernelOverloadMock.sol (95%) rename contracts/test/mocks/{ => kernel}/KernelSetAppMock.sol (89%) rename contracts/test/mocks/{ => kernel}/UpgradedKernel.sol (90%) rename contracts/test/mocks/{ => lib/math}/SafeMath64Mock.sol (93%) rename contracts/test/mocks/{ => lib/math}/SafeMath8Mock.sol (93%) rename contracts/test/mocks/{ => lib/misc}/ERCProxyMock.sol (88%) rename contracts/test/mocks/{ => lib/token}/SafeERC20Mock.sol (92%) rename contracts/test/mocks/{ => lib/token}/TokenMock.sol (98%) rename contracts/test/mocks/{ => lib/token}/TokenReturnFalseMock.sol (100%) rename contracts/test/mocks/{ => lib/token}/TokenReturnMissingMock.sol (98%) rename contracts/test/{ => tests}/TestACLInterpreter.sol (99%) rename contracts/test/{ => tests}/TestConversionHelpers.sol (98%) rename contracts/test/{ => tests}/TestDelegateProxy.sol (92%) delete mode 100644 test/acl_interpreter.js create mode 100644 test/contracts/acl/acl_interpreter.js rename test/{ => contracts/apm}/apm_namehash.js (100%) rename test/{ => contracts/apm}/apm_registry.js (99%) rename test/{ => contracts/apm}/apm_repo.js (98%) rename test/{ => contracts/apps}/app_acl.js (97%) rename test/{ => contracts/apps}/app_base_lifecycle.js (97%) rename test/{ => contracts/apps}/app_funds.js (96%) rename test/{ => contracts/apps}/app_proxy.js (98%) rename test/{ => contracts/apps}/recovery_to_vault.js (98%) create mode 100644 test/contracts/common/conversion_helpers.js create mode 100644 test/contracts/common/delegate_proxy.js rename test/{ => contracts/common}/keccak_constants.js (100%) rename test/{ => contracts/common}/lib_uint256_helpers.js (89%) rename test/{ => contracts/common}/lifecycle.js (92%) rename test/{ => contracts/common}/reentrancy_guard.js (98%) rename test/{ => contracts/common}/time_helpers.js (100%) rename test/{ => contracts/common}/unstructured_storage.js (100%) rename test/{ => contracts/ens}/ens_subdomains.js (98%) rename test/{ => contracts/evmscript}/evm_script.js (99%) rename test/{ => contracts/factory}/evm_script_factory.js (98%) rename test/{ => contracts/kernel}/kernel_acl.js (99%) rename test/{ => contracts/kernel}/kernel_apps.js (98%) rename test/{ => contracts/kernel}/kernel_funds.js (95%) rename test/{ => contracts/kernel}/kernel_lifecycle.js (97%) rename test/{ => contracts/kernel}/kernel_upgrade.js (96%) rename test/{ => contracts/lib/math}/lib_safemath64.js (96%) rename test/{ => contracts/lib/math}/lib_safemath8.js (96%) rename test/{ => contracts/lib/token}/safe_erc20.js (98%) delete mode 100644 test/conversion_helpers.js delete mode 100644 test/delegate_proxy.js diff --git a/contracts/test/mocks/APMNamehashMock.sol b/contracts/test/mocks/apm/APMNamehashMock.sol similarity index 87% rename from contracts/test/mocks/APMNamehashMock.sol rename to contracts/test/mocks/apm/APMNamehashMock.sol index d7a7c11a4..f741d19a9 100644 --- a/contracts/test/mocks/APMNamehashMock.sol +++ b/contracts/test/mocks/apm/APMNamehashMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../apm/APMNamehash.sol"; +import "../../../apm/APMNamehash.sol"; contract APMNamehashMock is APMNamehash { diff --git a/contracts/test/mocks/UnsafeRepo.sol b/contracts/test/mocks/apm/UnsafeRepo.sol similarity index 79% rename from contracts/test/mocks/UnsafeRepo.sol rename to contracts/test/mocks/apm/UnsafeRepo.sol index ce121b9c0..129d67f7c 100644 --- a/contracts/test/mocks/UnsafeRepo.sol +++ b/contracts/test/mocks/apm/UnsafeRepo.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../apm/Repo.sol"; -import "../../apps/UnsafeAragonApp.sol"; +import "../../../apm/Repo.sol"; +import "../../../apps/UnsafeAragonApp.sol"; // Allows Repo to be used without a proxy or access controls diff --git a/contracts/test/mocks/AppProxyPinnedStorageMock.sol b/contracts/test/mocks/apps/AppProxyPinnedStorageMock.sol similarity index 90% rename from contracts/test/mocks/AppProxyPinnedStorageMock.sol rename to contracts/test/mocks/apps/AppProxyPinnedStorageMock.sol index b64a47f50..1c4ea2a11 100644 --- a/contracts/test/mocks/AppProxyPinnedStorageMock.sol +++ b/contracts/test/mocks/apps/AppProxyPinnedStorageMock.sol @@ -1,8 +1,8 @@ pragma solidity 0.4.24; -import "../../apps/AppProxyPinned.sol"; -import "../../kernel/IKernel.sol"; -import "../../kernel/Kernel.sol"; +import "../../../apps/AppProxyPinned.sol"; +import "../../../kernel/IKernel.sol"; +import "../../../kernel/Kernel.sol"; contract FakeAppConstants { diff --git a/contracts/test/mocks/AppStorageMock.sol b/contracts/test/mocks/apps/AppStorageMock.sol similarity index 91% rename from contracts/test/mocks/AppStorageMock.sol rename to contracts/test/mocks/apps/AppStorageMock.sol index 53d977e69..4b7958d96 100644 --- a/contracts/test/mocks/AppStorageMock.sol +++ b/contracts/test/mocks/apps/AppStorageMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../apps/AppStorage.sol"; +import "../../../apps/AppStorage.sol"; contract AppStorageMock is AppStorage { diff --git a/contracts/test/mocks/AppStub.sol b/contracts/test/mocks/apps/AppStub.sol similarity index 88% rename from contracts/test/mocks/AppStub.sol rename to contracts/test/mocks/apps/AppStub.sol index 69a8efb35..e2a4acb00 100644 --- a/contracts/test/mocks/AppStub.sol +++ b/contracts/test/mocks/apps/AppStub.sol @@ -1,8 +1,8 @@ pragma solidity 0.4.24; -import "../../apps/AragonApp.sol"; -import "../../apps/UnsafeAragonApp.sol"; -import "../../kernel/IKernel.sol"; +import "../../../apps/AragonApp.sol"; +import "../../../apps/UnsafeAragonApp.sol"; +import "../../../kernel/IKernel.sol"; contract AppStubStorage { diff --git a/contracts/test/mocks/AppStubConditionalRecovery.sol b/contracts/test/mocks/apps/AppStubConditionalRecovery.sol similarity index 80% rename from contracts/test/mocks/AppStubConditionalRecovery.sol rename to contracts/test/mocks/apps/AppStubConditionalRecovery.sol index 65298671d..9ba879c2f 100644 --- a/contracts/test/mocks/AppStubConditionalRecovery.sol +++ b/contracts/test/mocks/apps/AppStubConditionalRecovery.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../apps/AragonApp.sol"; -import "../../common/DepositableStorage.sol"; +import "../../../apps/AragonApp.sol"; +import "../../../common/DepositableStorage.sol"; contract AppStubConditionalRecovery is AragonApp, DepositableStorage { diff --git a/contracts/test/mocks/AppStubDepositable.sol b/contracts/test/mocks/apps/AppStubDepositable.sol similarity index 78% rename from contracts/test/mocks/AppStubDepositable.sol rename to contracts/test/mocks/apps/AppStubDepositable.sol index 095b89ee5..17bdbd648 100644 --- a/contracts/test/mocks/AppStubDepositable.sol +++ b/contracts/test/mocks/apps/AppStubDepositable.sol @@ -1,8 +1,8 @@ pragma solidity 0.4.24; -import "../../apps/AragonApp.sol"; -import "../../apps/UnsafeAragonApp.sol"; -import "../../common/DepositableStorage.sol"; +import "../../../apps/AragonApp.sol"; +import "../../../apps/UnsafeAragonApp.sol"; +import "../../../common/DepositableStorage.sol"; contract AppStubDepositable is AragonApp, DepositableStorage { diff --git a/contracts/test/mocks/AppStubScriptRunner.sol b/contracts/test/mocks/apps/AppStubScriptRunner.sol similarity index 98% rename from contracts/test/mocks/AppStubScriptRunner.sol rename to contracts/test/mocks/apps/AppStubScriptRunner.sol index 6fcf54aa6..25efa04b1 100644 --- a/contracts/test/mocks/AppStubScriptRunner.sol +++ b/contracts/test/mocks/apps/AppStubScriptRunner.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../apps/AragonApp.sol"; +import "../../../apps/AragonApp.sol"; contract AppStubScriptRunner is AragonApp { diff --git a/contracts/test/mocks/UnsafeAragonAppMock.sol b/contracts/test/mocks/apps/UnsafeAragonAppMock.sol similarity index 79% rename from contracts/test/mocks/UnsafeAragonAppMock.sol rename to contracts/test/mocks/apps/UnsafeAragonAppMock.sol index 61551949c..7501e6212 100644 --- a/contracts/test/mocks/UnsafeAragonAppMock.sol +++ b/contracts/test/mocks/apps/UnsafeAragonAppMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../apps/UnsafeAragonApp.sol"; -import "../../kernel/IKernel.sol"; +import "../../../apps/UnsafeAragonApp.sol"; +import "../../../kernel/IKernel.sol"; contract UnsafeAragonAppMock is UnsafeAragonApp { diff --git a/contracts/test/mocks/VaultMock.sol b/contracts/test/mocks/apps/VaultMock.sol similarity index 77% rename from contracts/test/mocks/VaultMock.sol rename to contracts/test/mocks/apps/VaultMock.sol index 452f8ea25..2e9ead9c9 100644 --- a/contracts/test/mocks/VaultMock.sol +++ b/contracts/test/mocks/apps/VaultMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../apps/UnsafeAragonApp.sol"; -import "../../common/DepositableStorage.sol"; +import "../../../apps/UnsafeAragonApp.sol"; +import "../../../common/DepositableStorage.sol"; contract VaultMock is UnsafeAragonApp, DepositableStorage { diff --git a/contracts/test/mocks/DepositableStorageMock.sol b/contracts/test/mocks/common/DepositableStorageMock.sol similarity index 86% rename from contracts/test/mocks/DepositableStorageMock.sol rename to contracts/test/mocks/common/DepositableStorageMock.sol index ede41cde5..56f190a3b 100644 --- a/contracts/test/mocks/DepositableStorageMock.sol +++ b/contracts/test/mocks/common/DepositableStorageMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../common/DepositableStorage.sol"; +import "../../../common/DepositableStorage.sol"; contract DepositableStorageMock is DepositableStorage { diff --git a/contracts/test/mocks/EtherTokenConstantMock.sol b/contracts/test/mocks/common/EtherTokenConstantMock.sol similarity index 76% rename from contracts/test/mocks/EtherTokenConstantMock.sol rename to contracts/test/mocks/common/EtherTokenConstantMock.sol index 4174b58bc..0c7e8270b 100644 --- a/contracts/test/mocks/EtherTokenConstantMock.sol +++ b/contracts/test/mocks/common/EtherTokenConstantMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../common/EtherTokenConstant.sol"; +import "../../../common/EtherTokenConstant.sol"; contract EtherTokenConstantMock is EtherTokenConstant { diff --git a/contracts/test/mocks/InitializableStorageMock.sol b/contracts/test/mocks/common/InitializableStorageMock.sol similarity index 86% rename from contracts/test/mocks/InitializableStorageMock.sol rename to contracts/test/mocks/common/InitializableStorageMock.sol index 7dd93f1a2..a011c8825 100644 --- a/contracts/test/mocks/InitializableStorageMock.sol +++ b/contracts/test/mocks/common/InitializableStorageMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../common/Initializable.sol"; +import "../../../common/Initializable.sol"; contract InitializableStorageMock is Initializable { diff --git a/contracts/test/mocks/KeccakConstants.sol b/contracts/test/mocks/common/KeccakConstants.sol similarity index 100% rename from contracts/test/mocks/KeccakConstants.sol rename to contracts/test/mocks/common/KeccakConstants.sol diff --git a/contracts/test/mocks/LifecycleMock.sol b/contracts/test/mocks/common/LifecycleMock.sol similarity index 71% rename from contracts/test/mocks/LifecycleMock.sol rename to contracts/test/mocks/common/LifecycleMock.sol index bb52fcb85..885b00369 100644 --- a/contracts/test/mocks/LifecycleMock.sol +++ b/contracts/test/mocks/common/LifecycleMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../common/Initializable.sol"; -import "../../common/Petrifiable.sol"; +import "../../../common/Initializable.sol"; +import "../../../common/Petrifiable.sol"; contract LifecycleMock is Initializable, Petrifiable { diff --git a/contracts/test/mocks/ReentrancyGuardMock.sol b/contracts/test/mocks/common/ReentrancyGuardMock.sol similarity index 93% rename from contracts/test/mocks/ReentrancyGuardMock.sol rename to contracts/test/mocks/common/ReentrancyGuardMock.sol index 45a805ac0..84568b9e8 100644 --- a/contracts/test/mocks/ReentrancyGuardMock.sol +++ b/contracts/test/mocks/common/ReentrancyGuardMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../common/ReentrancyGuard.sol"; -import "../../common/UnstructuredStorage.sol"; +import "../../../common/ReentrancyGuard.sol"; +import "../../../common/UnstructuredStorage.sol"; contract ReentrantActor { diff --git a/contracts/test/mocks/TimeHelpersMock.sol b/contracts/test/mocks/common/TimeHelpersMock.sol similarity index 94% rename from contracts/test/mocks/TimeHelpersMock.sol rename to contracts/test/mocks/common/TimeHelpersMock.sol index d8bf3234b..af3894a75 100644 --- a/contracts/test/mocks/TimeHelpersMock.sol +++ b/contracts/test/mocks/common/TimeHelpersMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../common/TimeHelpers.sol"; +import "../../../common/TimeHelpers.sol"; contract TimeHelpersMock is TimeHelpers { diff --git a/contracts/test/mocks/Uint256Mock.sol b/contracts/test/mocks/common/Uint256Mock.sol similarity index 80% rename from contracts/test/mocks/Uint256Mock.sol rename to contracts/test/mocks/common/Uint256Mock.sol index 9616376d7..b323c648f 100644 --- a/contracts/test/mocks/Uint256Mock.sol +++ b/contracts/test/mocks/common/Uint256Mock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../common/Uint256Helpers.sol"; +import "../../../common/Uint256Helpers.sol"; contract Uint256Mock { diff --git a/contracts/test/mocks/ENSConstantsMock.sol b/contracts/test/mocks/ens/ENSConstantsMock.sol similarity index 93% rename from contracts/test/mocks/ENSConstantsMock.sol rename to contracts/test/mocks/ens/ENSConstantsMock.sol index d5d14a4c6..2c33a35bb 100644 --- a/contracts/test/mocks/ENSConstantsMock.sol +++ b/contracts/test/mocks/ens/ENSConstantsMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../ens/ENSConstants.sol"; +import "../../../ens/ENSConstants.sol"; contract ENSConstantsMock is ENSConstants { diff --git a/contracts/test/mocks/EVMScriptExecutorMock.sol b/contracts/test/mocks/evmscript/EVMScriptExecutorMock.sol similarity index 89% rename from contracts/test/mocks/EVMScriptExecutorMock.sol rename to contracts/test/mocks/evmscript/EVMScriptExecutorMock.sol index 3db26fe82..8ccbd2232 100644 --- a/contracts/test/mocks/EVMScriptExecutorMock.sol +++ b/contracts/test/mocks/evmscript/EVMScriptExecutorMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../evmscript/executors/BaseEVMScriptExecutor.sol"; +import "../../../evmscript/executors/BaseEVMScriptExecutor.sol"; contract EVMScriptExecutorMock is BaseEVMScriptExecutor { bytes32 internal constant EXECUTOR_TYPE = keccak256("MOCK_SCRIPT"); diff --git a/contracts/test/mocks/EVMScriptExecutorNoReturnMock.sol b/contracts/test/mocks/evmscript/EVMScriptExecutorNoReturnMock.sol similarity index 86% rename from contracts/test/mocks/EVMScriptExecutorNoReturnMock.sol rename to contracts/test/mocks/evmscript/EVMScriptExecutorNoReturnMock.sol index ecf84cd6b..81876dc31 100644 --- a/contracts/test/mocks/EVMScriptExecutorNoReturnMock.sol +++ b/contracts/test/mocks/evmscript/EVMScriptExecutorNoReturnMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../evmscript/executors/BaseEVMScriptExecutor.sol"; +import "../../../evmscript/executors/BaseEVMScriptExecutor.sol"; contract EVMScriptExecutorNoReturnMock is BaseEVMScriptExecutor { bytes32 internal constant EXECUTOR_TYPE = keccak256("NO_RETURN_SCRIPT"); diff --git a/contracts/test/mocks/EVMScriptExecutorRevertMock.sol b/contracts/test/mocks/evmscript/EVMScriptExecutorRevertMock.sol similarity index 87% rename from contracts/test/mocks/EVMScriptExecutorRevertMock.sol rename to contracts/test/mocks/evmscript/EVMScriptExecutorRevertMock.sol index 9ee5dd5b7..22e075f5a 100644 --- a/contracts/test/mocks/EVMScriptExecutorRevertMock.sol +++ b/contracts/test/mocks/evmscript/EVMScriptExecutorRevertMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../evmscript/executors/BaseEVMScriptExecutor.sol"; +import "../../../evmscript/executors/BaseEVMScriptExecutor.sol"; contract EVMScriptExecutorRevertMock is BaseEVMScriptExecutor { string public constant ERROR_MOCK_REVERT = "MOCK_REVERT"; diff --git a/contracts/test/mocks/EVMScriptRegistryConstantsMock.sol b/contracts/test/mocks/evmscript/EVMScriptRegistryConstantsMock.sol similarity index 80% rename from contracts/test/mocks/EVMScriptRegistryConstantsMock.sol rename to contracts/test/mocks/evmscript/EVMScriptRegistryConstantsMock.sol index 87c608cb3..4ca8dfb30 100644 --- a/contracts/test/mocks/EVMScriptRegistryConstantsMock.sol +++ b/contracts/test/mocks/evmscript/EVMScriptRegistryConstantsMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../evmscript/IEVMScriptRegistry.sol"; +import "../../../evmscript/IEVMScriptRegistry.sol"; contract EVMScriptRegistryConstantsMock is EVMScriptRegistryConstants { diff --git a/contracts/test/mocks/ExecutionTarget.sol b/contracts/test/mocks/evmscript/ExecutionTarget.sol similarity index 100% rename from contracts/test/mocks/ExecutionTarget.sol rename to contracts/test/mocks/evmscript/ExecutionTarget.sol diff --git a/contracts/test/mocks/APMRegistryFactoryMock.sol b/contracts/test/mocks/factory/APMRegistryFactoryMock.sol similarity index 93% rename from contracts/test/mocks/APMRegistryFactoryMock.sol rename to contracts/test/mocks/factory/APMRegistryFactoryMock.sol index 455948b59..594cea916 100644 --- a/contracts/test/mocks/APMRegistryFactoryMock.sol +++ b/contracts/test/mocks/factory/APMRegistryFactoryMock.sol @@ -1,11 +1,11 @@ pragma solidity 0.4.24; -import "../../apm/APMRegistry.sol"; -import "../../apm/Repo.sol"; -import "../../ens/ENSSubdomainRegistrar.sol"; +import "../../../apm/APMRegistry.sol"; +import "../../../apm/Repo.sol"; +import "../../../ens/ENSSubdomainRegistrar.sol"; -import "../../factory/DAOFactory.sol"; -import "../../factory/ENSFactory.sol"; +import "../../../factory/DAOFactory.sol"; +import "../../../factory/ENSFactory.sol"; // Mock that doesn't grant enough permissions // Only usable with new ENS instance diff --git a/contracts/test/mocks/KernelConstantsMock.sol b/contracts/test/mocks/kernel/KernelConstantsMock.sol similarity index 95% rename from contracts/test/mocks/KernelConstantsMock.sol rename to contracts/test/mocks/kernel/KernelConstantsMock.sol index 10371ba04..47c634848 100644 --- a/contracts/test/mocks/KernelConstantsMock.sol +++ b/contracts/test/mocks/kernel/KernelConstantsMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../kernel/Kernel.sol"; +import "../../../kernel/Kernel.sol"; contract KernelConstantsMock is Kernel { diff --git a/contracts/test/mocks/KernelDepositableMock.sol b/contracts/test/mocks/kernel/KernelDepositableMock.sol similarity index 79% rename from contracts/test/mocks/KernelDepositableMock.sol rename to contracts/test/mocks/kernel/KernelDepositableMock.sol index cf0e16f11..4de98fbb1 100644 --- a/contracts/test/mocks/KernelDepositableMock.sol +++ b/contracts/test/mocks/kernel/KernelDepositableMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../common/DepositableStorage.sol"; -import "../../kernel/Kernel.sol"; +import "../../../common/DepositableStorage.sol"; +import "../../../kernel/Kernel.sol"; contract KernelDepositableMock is Kernel, DepositableStorage { constructor(bool _shouldPetrify) Kernel(_shouldPetrify) public { diff --git a/contracts/test/mocks/KernelOverloadMock.sol b/contracts/test/mocks/kernel/KernelOverloadMock.sol similarity index 95% rename from contracts/test/mocks/KernelOverloadMock.sol rename to contracts/test/mocks/kernel/KernelOverloadMock.sol index dfaa94c57..66d87ec2b 100644 --- a/contracts/test/mocks/KernelOverloadMock.sol +++ b/contracts/test/mocks/kernel/KernelOverloadMock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../kernel/Kernel.sol"; -import "../../lib/misc/ERCProxy.sol"; +import "../../../kernel/Kernel.sol"; +import "../../../lib/misc/ERCProxy.sol"; /** Ugly hack to work around this issue: diff --git a/contracts/test/mocks/KernelSetAppMock.sol b/contracts/test/mocks/kernel/KernelSetAppMock.sol similarity index 89% rename from contracts/test/mocks/KernelSetAppMock.sol rename to contracts/test/mocks/kernel/KernelSetAppMock.sol index df713cee4..2b91a6449 100644 --- a/contracts/test/mocks/KernelSetAppMock.sol +++ b/contracts/test/mocks/kernel/KernelSetAppMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../kernel/Kernel.sol"; +import "../../../kernel/Kernel.sol"; contract KernelSetAppMock is Kernel { constructor() Kernel(false) public { diff --git a/contracts/test/mocks/UpgradedKernel.sol b/contracts/test/mocks/kernel/UpgradedKernel.sol similarity index 90% rename from contracts/test/mocks/UpgradedKernel.sol rename to contracts/test/mocks/kernel/UpgradedKernel.sol index 5a00fd2a5..2f84bc802 100644 --- a/contracts/test/mocks/UpgradedKernel.sol +++ b/contracts/test/mocks/kernel/UpgradedKernel.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../kernel/Kernel.sol"; +import "../../../kernel/Kernel.sol"; contract UpgradedKernel is Kernel { diff --git a/contracts/test/mocks/SafeMath64Mock.sol b/contracts/test/mocks/lib/math/SafeMath64Mock.sol similarity index 93% rename from contracts/test/mocks/SafeMath64Mock.sol rename to contracts/test/mocks/lib/math/SafeMath64Mock.sol index bd1fec780..de3bed0ec 100644 --- a/contracts/test/mocks/SafeMath64Mock.sol +++ b/contracts/test/mocks/lib/math/SafeMath64Mock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../lib/math/SafeMath64.sol"; +import "../../../../lib/math/SafeMath64.sol"; contract SafeMath64Mock { diff --git a/contracts/test/mocks/SafeMath8Mock.sol b/contracts/test/mocks/lib/math/SafeMath8Mock.sol similarity index 93% rename from contracts/test/mocks/SafeMath8Mock.sol rename to contracts/test/mocks/lib/math/SafeMath8Mock.sol index 7105f0ee9..df22aebaf 100644 --- a/contracts/test/mocks/SafeMath8Mock.sol +++ b/contracts/test/mocks/lib/math/SafeMath8Mock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../lib/math/SafeMath8.sol"; +import "../../../../lib/math/SafeMath8.sol"; contract SafeMath8Mock { diff --git a/contracts/test/mocks/ERCProxyMock.sol b/contracts/test/mocks/lib/misc/ERCProxyMock.sol similarity index 88% rename from contracts/test/mocks/ERCProxyMock.sol rename to contracts/test/mocks/lib/misc/ERCProxyMock.sol index 3b0bd6550..a85da5f29 100644 --- a/contracts/test/mocks/ERCProxyMock.sol +++ b/contracts/test/mocks/lib/misc/ERCProxyMock.sol @@ -1,6 +1,6 @@ pragma solidity 0.4.24; -import "../../lib/misc/ERCProxy.sol"; +import "../../../../lib/misc/ERCProxy.sol"; contract ERCProxyMock is ERCProxy { diff --git a/contracts/test/mocks/SafeERC20Mock.sol b/contracts/test/mocks/lib/token/SafeERC20Mock.sol similarity index 92% rename from contracts/test/mocks/SafeERC20Mock.sol rename to contracts/test/mocks/lib/token/SafeERC20Mock.sol index 92b660ced..9f13848f6 100644 --- a/contracts/test/mocks/SafeERC20Mock.sol +++ b/contracts/test/mocks/lib/token/SafeERC20Mock.sol @@ -1,7 +1,7 @@ pragma solidity 0.4.24; -import "../../common/SafeERC20.sol"; -import "../../lib/token/ERC20.sol"; +import "../../../../common/SafeERC20.sol"; +import "../../../../lib/token/ERC20.sol"; contract SafeERC20Mock { diff --git a/contracts/test/mocks/TokenMock.sol b/contracts/test/mocks/lib/token/TokenMock.sol similarity index 98% rename from contracts/test/mocks/TokenMock.sol rename to contracts/test/mocks/lib/token/TokenMock.sol index a93f777aa..8d5174452 100644 --- a/contracts/test/mocks/TokenMock.sol +++ b/contracts/test/mocks/lib/token/TokenMock.sol @@ -2,7 +2,7 @@ pragma solidity 0.4.24; -import "../../lib/math/SafeMath.sol"; +import "../../../../lib/math/SafeMath.sol"; contract TokenMock { diff --git a/contracts/test/mocks/TokenReturnFalseMock.sol b/contracts/test/mocks/lib/token/TokenReturnFalseMock.sol similarity index 100% rename from contracts/test/mocks/TokenReturnFalseMock.sol rename to contracts/test/mocks/lib/token/TokenReturnFalseMock.sol diff --git a/contracts/test/mocks/TokenReturnMissingMock.sol b/contracts/test/mocks/lib/token/TokenReturnMissingMock.sol similarity index 98% rename from contracts/test/mocks/TokenReturnMissingMock.sol rename to contracts/test/mocks/lib/token/TokenReturnMissingMock.sol index 8b339b1c8..6668da294 100644 --- a/contracts/test/mocks/TokenReturnMissingMock.sol +++ b/contracts/test/mocks/lib/token/TokenReturnMissingMock.sol @@ -4,7 +4,7 @@ pragma solidity 0.4.24; -import "../../lib/math/SafeMath.sol"; +import "../../../../lib/math/SafeMath.sol"; contract TokenReturnMissingMock { diff --git a/contracts/test/TestACLInterpreter.sol b/contracts/test/tests/TestACLInterpreter.sol similarity index 99% rename from contracts/test/TestACLInterpreter.sol rename to contracts/test/tests/TestACLInterpreter.sol index ca909e91e..b4441d2a4 100644 --- a/contracts/test/TestACLInterpreter.sol +++ b/contracts/test/tests/TestACLInterpreter.sol @@ -1,8 +1,8 @@ pragma solidity 0.4.24; -import "../acl/ACL.sol"; -import "./helpers/Assert.sol"; -import "./helpers/ACLHelper.sol"; +import "../../acl/ACL.sol"; +import "../helpers/Assert.sol"; +import "../helpers/ACLHelper.sol"; contract TestACLInterpreter is ACL, ACLHelper { diff --git a/contracts/test/TestConversionHelpers.sol b/contracts/test/tests/TestConversionHelpers.sol similarity index 98% rename from contracts/test/TestConversionHelpers.sol rename to contracts/test/tests/TestConversionHelpers.sol index 8593c5684..ea7da4c6f 100644 --- a/contracts/test/TestConversionHelpers.sol +++ b/contracts/test/tests/TestConversionHelpers.sol @@ -1,9 +1,9 @@ pragma solidity 0.4.24; -import "./helpers/Assert.sol"; -import "./helpers/ThrowProxy.sol"; +import "../helpers/Assert.sol"; +import "../helpers/ThrowProxy.sol"; -import "../common/ConversionHelpers.sol"; +import "../../common/ConversionHelpers.sol"; contract InvalidBytesLengthConversionThrows { diff --git a/contracts/test/TestDelegateProxy.sol b/contracts/test/tests/TestDelegateProxy.sol similarity index 92% rename from contracts/test/TestDelegateProxy.sol rename to contracts/test/tests/TestDelegateProxy.sol index adce13545..1ba5a836a 100644 --- a/contracts/test/TestDelegateProxy.sol +++ b/contracts/test/tests/TestDelegateProxy.sol @@ -1,10 +1,10 @@ pragma solidity 0.4.24; -import "./helpers/Assert.sol"; -import "./helpers/ThrowProxy.sol"; +import "../helpers/Assert.sol"; +import "../helpers/ThrowProxy.sol"; -import "../common/DelegateProxy.sol"; -import "../evmscript/ScriptHelpers.sol"; +import "../../common/DelegateProxy.sol"; +import "../../evmscript/ScriptHelpers.sol"; contract Target { diff --git a/test/acl_interpreter.js b/test/acl_interpreter.js deleted file mode 100644 index 4422dcf6c..000000000 --- a/test/acl_interpreter.js +++ /dev/null @@ -1,3 +0,0 @@ -const runSolidityTest = require('./helpers/runSolidityTest') - -runSolidityTest('TestACLInterpreter') diff --git a/test/contracts/acl/acl_interpreter.js b/test/contracts/acl/acl_interpreter.js new file mode 100644 index 000000000..d3e0baf47 --- /dev/null +++ b/test/contracts/acl/acl_interpreter.js @@ -0,0 +1,3 @@ +const runSolidityTest = require('../../helpers/runSolidityTest') + +runSolidityTest('TestACLInterpreter') diff --git a/test/apm_namehash.js b/test/contracts/apm/apm_namehash.js similarity index 100% rename from test/apm_namehash.js rename to test/contracts/apm/apm_namehash.js diff --git a/test/apm_registry.js b/test/contracts/apm/apm_registry.js similarity index 99% rename from test/apm_registry.js rename to test/contracts/apm/apm_registry.js index 286f1da1d..876a5ec8d 100644 --- a/test/apm_registry.js +++ b/test/contracts/apm/apm_registry.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../helpers/assertThrow') const namehash = require('eth-ens-namehash').hash const keccak256 = require('js-sha3').keccak_256 diff --git a/test/apm_repo.js b/test/contracts/apm/apm_repo.js similarity index 98% rename from test/apm_repo.js rename to test/contracts/apm/apm_repo.js index df46b892e..900023fed 100644 --- a/test/apm_repo.js +++ b/test/contracts/apm/apm_repo.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../helpers/assertThrow') const Repo = artifacts.require('UnsafeRepo') diff --git a/test/app_acl.js b/test/contracts/apps/app_acl.js similarity index 97% rename from test/app_acl.js rename to test/contracts/apps/app_acl.js index f90d4ec21..cd0549f02 100644 --- a/test/app_acl.js +++ b/test/contracts/apps/app_acl.js @@ -1,5 +1,5 @@ -const { assertRevert } = require('./helpers/assertThrow') -const { onlyIf } = require('./helpers/onlyIf') +const { assertRevert } = require('../../helpers/assertThrow') +const { onlyIf } = require('../../helpers/onlyIf') const { hash } = require('eth-ens-namehash') const ACL = artifacts.require('ACL') diff --git a/test/app_base_lifecycle.js b/test/contracts/apps/app_base_lifecycle.js similarity index 97% rename from test/app_base_lifecycle.js rename to test/contracts/apps/app_base_lifecycle.js index 000823945..94ee80438 100644 --- a/test/app_base_lifecycle.js +++ b/test/contracts/apps/app_base_lifecycle.js @@ -1,5 +1,5 @@ -const { assertRevert } = require('./helpers/assertThrow') -const { getBlockNumber } = require('./helpers/web3') +const { assertRevert } = require('../../helpers/assertThrow') +const { getBlockNumber } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') const { soliditySha3 } = require('web3-utils') diff --git a/test/app_funds.js b/test/contracts/apps/app_funds.js similarity index 96% rename from test/app_funds.js rename to test/contracts/apps/app_funds.js index 152c984a5..b001d9ea4 100644 --- a/test/app_funds.js +++ b/test/contracts/apps/app_funds.js @@ -1,7 +1,7 @@ const { hash } = require('eth-ens-namehash') -const { assertRevert } = require('./helpers/assertThrow') -const { getBalance } = require('./helpers/web3') -const { onlyIf } = require('./helpers/onlyIf') +const { assertRevert } = require('../../helpers/assertThrow') +const { getBalance } = require('../../helpers/web3') +const { onlyIf } = require('../../helpers/onlyIf') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') diff --git a/test/app_proxy.js b/test/contracts/apps/app_proxy.js similarity index 98% rename from test/app_proxy.js rename to test/contracts/apps/app_proxy.js index d2d5df4b3..2e3ea991c 100644 --- a/test/app_proxy.js +++ b/test/contracts/apps/app_proxy.js @@ -1,6 +1,6 @@ -const { assertRevert } = require('./helpers/assertThrow') -const { onlyIf } = require('./helpers/onlyIf') -const { getBlockNumber } = require('./helpers/web3') +const { assertRevert } = require('../../helpers/assertThrow') +const { onlyIf } = require('../../helpers/onlyIf') +const { getBlockNumber } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') const ACL = artifacts.require('ACL') diff --git a/test/recovery_to_vault.js b/test/contracts/apps/recovery_to_vault.js similarity index 98% rename from test/recovery_to_vault.js rename to test/contracts/apps/recovery_to_vault.js index 5fbb823a9..76dd7ddb9 100644 --- a/test/recovery_to_vault.js +++ b/test/contracts/apps/recovery_to_vault.js @@ -1,7 +1,7 @@ -const assertEvent = require('./helpers/assertEvent') -const { assertRevert } = require('./helpers/assertThrow') -const { skipCoverage } = require('./helpers/coverage') -const { getBalance } = require('./helpers/web3') +const assertEvent = require('../../helpers/assertEvent') +const { assertRevert } = require('../../helpers/assertThrow') +const { skipCoverage } = require('../../helpers/coverage') +const { getBalance } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') const DAOFactory = artifacts.require('DAOFactory') diff --git a/test/contracts/common/conversion_helpers.js b/test/contracts/common/conversion_helpers.js new file mode 100644 index 000000000..45d83e959 --- /dev/null +++ b/test/contracts/common/conversion_helpers.js @@ -0,0 +1,3 @@ +const runSolidityTest = require('../../helpers/runSolidityTest') + +runSolidityTest('TestConversionHelpers') diff --git a/test/contracts/common/delegate_proxy.js b/test/contracts/common/delegate_proxy.js new file mode 100644 index 000000000..9f4f31031 --- /dev/null +++ b/test/contracts/common/delegate_proxy.js @@ -0,0 +1,3 @@ +const runSolidityTest = require('../../helpers/runSolidityTest') + +runSolidityTest('TestDelegateProxy') diff --git a/test/keccak_constants.js b/test/contracts/common/keccak_constants.js similarity index 100% rename from test/keccak_constants.js rename to test/contracts/common/keccak_constants.js diff --git a/test/lib_uint256_helpers.js b/test/contracts/common/lib_uint256_helpers.js similarity index 89% rename from test/lib_uint256_helpers.js rename to test/contracts/common/lib_uint256_helpers.js index 706f98e44..3173d957c 100644 --- a/test/lib_uint256_helpers.js +++ b/test/contracts/common/lib_uint256_helpers.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../helpers/assertThrow') contract('Uint256 Helpers test', accounts => { let uint256Mock diff --git a/test/lifecycle.js b/test/contracts/common/lifecycle.js similarity index 92% rename from test/lifecycle.js rename to test/contracts/common/lifecycle.js index ce72ef45d..cabe2f6f7 100644 --- a/test/lifecycle.js +++ b/test/contracts/common/lifecycle.js @@ -1,6 +1,6 @@ -const { assertRevert } = require('./helpers/assertThrow') -const { getBlockNumber } = require('./helpers/web3') -const reverts = require('./helpers/revertStrings') +const { assertRevert } = require('../../helpers/assertThrow') +const { getBlockNumber } = require('../../helpers/web3') +const reverts = require('../../helpers/revertStrings') // Mocks const LifecycleMock = artifacts.require('LifecycleMock') diff --git a/test/reentrancy_guard.js b/test/contracts/common/reentrancy_guard.js similarity index 98% rename from test/reentrancy_guard.js rename to test/contracts/common/reentrancy_guard.js index 1d0aecbe2..2e9944092 100644 --- a/test/reentrancy_guard.js +++ b/test/contracts/common/reentrancy_guard.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../helpers/assertThrow') const ZERO_ADDR = '0x0000000000000000000000000000000000000000' diff --git a/test/time_helpers.js b/test/contracts/common/time_helpers.js similarity index 100% rename from test/time_helpers.js rename to test/contracts/common/time_helpers.js diff --git a/test/unstructured_storage.js b/test/contracts/common/unstructured_storage.js similarity index 100% rename from test/unstructured_storage.js rename to test/contracts/common/unstructured_storage.js diff --git a/test/ens_subdomains.js b/test/contracts/ens/ens_subdomains.js similarity index 98% rename from test/ens_subdomains.js rename to test/contracts/ens/ens_subdomains.js index b3f251469..f3168f684 100644 --- a/test/ens_subdomains.js +++ b/test/contracts/ens/ens_subdomains.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../helpers/assertThrow') const namehash = require('eth-ens-namehash').hash const keccak256 = require('js-sha3').keccak_256 diff --git a/test/evm_script.js b/test/contracts/evmscript/evm_script.js similarity index 99% rename from test/evm_script.js rename to test/contracts/evmscript/evm_script.js index 28c802a9c..8f919421e 100644 --- a/test/evm_script.js +++ b/test/contracts/evmscript/evm_script.js @@ -1,10 +1,10 @@ const { rawEncode } = require('ethereumjs-abi') const { soliditySha3 } = require('web3-utils') -const assertEvent = require('./helpers/assertEvent') -const { assertRevert } = require('./helpers/assertThrow') -const { createExecutorId, encodeCallScript } = require('./helpers/evmScript') -const reverts = require('./helpers/revertStrings') +const assertEvent = require('../../helpers/assertEvent') +const { assertRevert } = require('../../helpers/assertThrow') +const { createExecutorId, encodeCallScript } = require('../../helpers/evmScript') +const reverts = require('../../helpers/revertStrings') const Kernel = artifacts.require('Kernel') const KernelProxy = artifacts.require('KernelProxy') diff --git a/test/evm_script_factory.js b/test/contracts/factory/evm_script_factory.js similarity index 98% rename from test/evm_script_factory.js rename to test/contracts/factory/evm_script_factory.js index 8a9934d11..4c81dd5a3 100644 --- a/test/evm_script_factory.js +++ b/test/contracts/factory/evm_script_factory.js @@ -1,5 +1,5 @@ const { soliditySha3 } = require('web3-utils') -const { createExecutorId, encodeCallScript } = require('./helpers/evmScript') +const { createExecutorId, encodeCallScript } = require('../../helpers/evmScript') const Kernel = artifacts.require('Kernel') const ACL = artifacts.require('ACL') diff --git a/test/kernel_acl.js b/test/contracts/kernel/kernel_acl.js similarity index 99% rename from test/kernel_acl.js rename to test/contracts/kernel/kernel_acl.js index 73e1bcb37..e40fc5450 100644 --- a/test/kernel_acl.js +++ b/test/contracts/kernel/kernel_acl.js @@ -1,5 +1,5 @@ -const assertEvent = require('./helpers/assertEvent') -const { assertRevert } = require('./helpers/assertThrow') +const assertEvent = require('../../helpers/assertEvent') +const { assertRevert } = require('../../helpers/assertThrow') const { hash } = require('eth-ens-namehash') const { soliditySha3 } = require('web3-utils') const keccak_256 = require('js-sha3').keccak_256 diff --git a/test/kernel_apps.js b/test/contracts/kernel/kernel_apps.js similarity index 98% rename from test/kernel_apps.js rename to test/contracts/kernel/kernel_apps.js index 995924a45..6e6610035 100644 --- a/test/kernel_apps.js +++ b/test/contracts/kernel/kernel_apps.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 { onlyIf } = require('../../helpers/onlyIf') +const { getBalance } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') const ACL = artifacts.require('ACL') diff --git a/test/kernel_funds.js b/test/contracts/kernel/kernel_funds.js similarity index 95% rename from test/kernel_funds.js rename to test/contracts/kernel/kernel_funds.js index 8096b8c6f..4f47636eb 100644 --- a/test/kernel_funds.js +++ b/test/contracts/kernel/kernel_funds.js @@ -1,6 +1,6 @@ -const { assertRevert } = require('./helpers/assertThrow') -const { getBalance } = require('./helpers/web3') -const { onlyIf } = require('./helpers/onlyIf') +const { assertRevert } = require('../../helpers/assertThrow') +const { getBalance } = require('../../helpers/web3') +const { onlyIf } = require('../../helpers/onlyIf') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') diff --git a/test/kernel_lifecycle.js b/test/contracts/kernel/kernel_lifecycle.js similarity index 97% rename from test/kernel_lifecycle.js rename to test/contracts/kernel/kernel_lifecycle.js index ae4bb3667..60c46007d 100644 --- a/test/kernel_lifecycle.js +++ b/test/contracts/kernel/kernel_lifecycle.js @@ -1,6 +1,6 @@ -const assertEvent = require('./helpers/assertEvent') -const { assertRevert } = require('./helpers/assertThrow') -const { getBlockNumber } = require('./helpers/web3') +const assertEvent = require('../../helpers/assertEvent') +const { assertRevert } = require('../../helpers/assertThrow') +const { getBlockNumber } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') const ACL = artifacts.require('ACL') diff --git a/test/kernel_upgrade.js b/test/contracts/kernel/kernel_upgrade.js similarity index 96% rename from test/kernel_upgrade.js rename to test/contracts/kernel/kernel_upgrade.js index db0d908b7..b7a997aa1 100644 --- a/test/kernel_upgrade.js +++ b/test/contracts/kernel/kernel_upgrade.js @@ -1,5 +1,5 @@ -const { assertRevert } = require('./helpers/assertThrow') -const { decodeEventsOfType } = require('./helpers/decodeEvent') +const { assertRevert } = require('../../helpers/assertThrow') +const { decodeEventsOfType } = require('../../helpers/decodeEvent') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') diff --git a/test/lib_safemath64.js b/test/contracts/lib/math/lib_safemath64.js similarity index 96% rename from test/lib_safemath64.js rename to test/contracts/lib/math/lib_safemath64.js index 86598406f..07a1b9b25 100644 --- a/test/lib_safemath64.js +++ b/test/contracts/lib/math/lib_safemath64.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../../helpers/assertThrow') contract('SafeMath64 lib test', accounts => { let safeMath64Mock diff --git a/test/lib_safemath8.js b/test/contracts/lib/math/lib_safemath8.js similarity index 96% rename from test/lib_safemath8.js rename to test/contracts/lib/math/lib_safemath8.js index f115d1576..e350fa8a4 100644 --- a/test/lib_safemath8.js +++ b/test/contracts/lib/math/lib_safemath8.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../../helpers/assertThrow') contract('SafeMath8 lib test', accounts => { let safeMath8Mock diff --git a/test/safe_erc20.js b/test/contracts/lib/token/safe_erc20.js similarity index 98% rename from test/safe_erc20.js rename to test/contracts/lib/token/safe_erc20.js index 60b173c20..cacfb26f8 100644 --- a/test/safe_erc20.js +++ b/test/contracts/lib/token/safe_erc20.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('./helpers/assertThrow') +const { assertRevert } = require('../../../helpers/assertThrow') // Mocks const SafeERC20Mock = artifacts.require('SafeERC20Mock') diff --git a/test/conversion_helpers.js b/test/conversion_helpers.js deleted file mode 100644 index b25f3010d..000000000 --- a/test/conversion_helpers.js +++ /dev/null @@ -1,3 +0,0 @@ -const runSolidityTest = require('./helpers/runSolidityTest') - -runSolidityTest('TestConversionHelpers') diff --git a/test/delegate_proxy.js b/test/delegate_proxy.js deleted file mode 100644 index c176df9f5..000000000 --- a/test/delegate_proxy.js +++ /dev/null @@ -1,3 +0,0 @@ -const runSolidityTest = require('./helpers/runSolidityTest') - -runSolidityTest('TestDelegateProxy') From 9071a60a5f77a3ccab2d13223aeb47af1420bd8f Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Fri, 3 May 2019 15:47:46 +0200 Subject: [PATCH 03/10] cosmetic: re-organize some library tests (#519) --- test/contracts/{lib/token => common}/safe_erc20.js | 2 +- .../common/{lib_uint256_helpers.js => uint256_helpers.js} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/contracts/{lib/token => common}/safe_erc20.js (98%) rename test/contracts/common/{lib_uint256_helpers.js => uint256_helpers.js} (100%) diff --git a/test/contracts/lib/token/safe_erc20.js b/test/contracts/common/safe_erc20.js similarity index 98% rename from test/contracts/lib/token/safe_erc20.js rename to test/contracts/common/safe_erc20.js index cacfb26f8..b92bfe5e5 100644 --- a/test/contracts/lib/token/safe_erc20.js +++ b/test/contracts/common/safe_erc20.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('../../../helpers/assertThrow') +const { assertRevert } = require('../../helpers/assertThrow') // Mocks const SafeERC20Mock = artifacts.require('SafeERC20Mock') diff --git a/test/contracts/common/lib_uint256_helpers.js b/test/contracts/common/uint256_helpers.js similarity index 100% rename from test/contracts/common/lib_uint256_helpers.js rename to test/contracts/common/uint256_helpers.js From 98e5e79f2d491bb53620439fb6ad78fbf103eeee Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Tue, 7 May 2019 15:56:08 -0300 Subject: [PATCH 04/10] helpers: fix assert revert helper (#522) --- test/helpers/assertThrow.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers/assertThrow.js b/test/helpers/assertThrow.js index f16d8a7fa..c15565328 100644 --- a/test/helpers/assertThrow.js +++ b/test/helpers/assertThrow.js @@ -32,7 +32,7 @@ module.exports = { } if (process.env.SOLIDITY_COVERAGE !== 'true' && reason) { - assert.equal(reason, error.reason, `Expected revert reason "${reason}" but failed with "${error.reason || 'no reason'}" instead.`) + assert.equal(error.reason, reason, `Expected revert reason "${reason}" but failed with "${error.reason || 'no reason'}" instead.`) } }, } From 220b6b1d85c40acf043cb428209306cf8c202f43 Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Wed, 8 May 2019 13:27:20 -0300 Subject: [PATCH 05/10] Tests: Add DAO factory unit tests (#524) --- test/contracts/factory/dao_factory.js | 120 ++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 test/contracts/factory/dao_factory.js diff --git a/test/contracts/factory/dao_factory.js b/test/contracts/factory/dao_factory.js new file mode 100644 index 000000000..c2603e3db --- /dev/null +++ b/test/contracts/factory/dao_factory.js @@ -0,0 +1,120 @@ +const DAOFactory = artifacts.require('DAOFactory') + +const ACL = artifacts.require('ACL') +const Kernel = artifacts.require('Kernel') +const EVMScriptRegistry = artifacts.require('EVMScriptRegistry') +const EVMScriptRegistryFactory = artifacts.require('EVMScriptRegistryFactory') +const EVMScriptRegistryConstants = artifacts.require('EVMScriptRegistryConstantsMock') + +const ZERO_ADDRES = '0x0000000000000000000000000000000000000000' + +const getEventArgument = (receipt, event, arg) => receipt.logs.filter(l => l.event === event)[0].args[arg] + +contract('DAO Factory', ([_, root]) => { + let daoFactory, dao, acl, receipt + + let CORE_NAMESPACE, APP_ADDR_NAMESPACE, APP_BASES_NAMESPACE + let APP_MANAGER_ROLE, CREATE_PERMISSIONS_ROLE, REGISTRY_ADD_EXECUTOR_ROLE + let ACL_APP_ID, KERNEL_APP_ID, EVM_SCRIPT_REGISTRY_APP_ID + let kernelBase, aclBase, scriptsRegistryFactory, scriptsRegistryBase, scriptsRegistryConstants + + before('deploy base implementations', async () => { + kernelBase = await Kernel.new(true) // petrify immediately + aclBase = await ACL.new() + scriptsRegistryFactory = await EVMScriptRegistryFactory.new() + scriptsRegistryConstants = await EVMScriptRegistryConstants.new() + scriptsRegistryBase = EVMScriptRegistry.at(await scriptsRegistryFactory.baseReg()) + }) + + before('load roles and constants', async () => { + ACL_APP_ID = await kernelBase.DEFAULT_ACL_APP_ID() + KERNEL_APP_ID = await kernelBase.KERNEL_APP_ID() + EVM_SCRIPT_REGISTRY_APP_ID = await scriptsRegistryConstants.getEVMScriptRegistryAppId() + + CORE_NAMESPACE = await kernelBase.CORE_NAMESPACE() + APP_ADDR_NAMESPACE = await kernelBase.APP_ADDR_NAMESPACE() + APP_BASES_NAMESPACE = await kernelBase.APP_BASES_NAMESPACE() + + APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE() + CREATE_PERMISSIONS_ROLE = await aclBase.CREATE_PERMISSIONS_ROLE() + REGISTRY_ADD_EXECUTOR_ROLE = await scriptsRegistryBase.REGISTRY_ADD_EXECUTOR_ROLE() + }) + + const itCreatesADao = () => { + it('creates a new DAO', async () => { + assert(await dao.hasInitialized(), 'DAO should be initialized') + assert.equal(await dao.getApp(CORE_NAMESPACE, KERNEL_APP_ID), kernelBase.address) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, ACL_APP_ID), aclBase.address) + assert.equal(await dao.getApp(APP_ADDR_NAMESPACE, ACL_APP_ID), acl.address) + }) + + it('sets the given root address as the permissions creator of the DAO', async () => { + assert(await acl.hasInitialized(), 'ACL should be initialized') + assert.equal(await acl.getPermissionManager(acl.address, CREATE_PERMISSIONS_ROLE), root) + assert.isTrue(await acl.hasPermission(root, acl.address, CREATE_PERMISSIONS_ROLE)) + assert.isFalse(await acl.hasPermission(daoFactory.address, acl.address, CREATE_PERMISSIONS_ROLE)) + }) + + it('does not create or grant app manager to the root address of the DAO', async () => { + assert.equal(await acl.getPermissionManager(dao.address, APP_MANAGER_ROLE), ZERO_ADDRES) + assert.isFalse(await acl.hasPermission(root, dao.address, APP_MANAGER_ROLE)) + assert.isFalse(await acl.hasPermission(daoFactory.address, dao.address, APP_MANAGER_ROLE)) + }) + } + + const itDoesCreateAnEVMScriptsRegistry = () => { + it('deploys an EVM script registry with a script executor', async () => { + const scriptsRegistry = EVMScriptRegistry.at(getEventArgument(receipt, 'DeployEVMScriptRegistry', 'reg')) + + assert(await scriptsRegistry.hasInitialized(), 'EVM scripts registry should be initialized') + assert.equal(await dao.getApp(APP_ADDR_NAMESPACE, EVM_SCRIPT_REGISTRY_APP_ID), scriptsRegistry.address) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, EVM_SCRIPT_REGISTRY_APP_ID), scriptsRegistryBase.address) + + const [executor] = await scriptsRegistry.executors(1) + assert.equal(executor, await scriptsRegistryFactory.baseCallScript()) + + assert.equal(await acl.getPermissionManager(scriptsRegistry.address, REGISTRY_ADD_EXECUTOR_ROLE), ZERO_ADDRES) + assert.isFalse(await acl.hasPermission(root, scriptsRegistry.address, REGISTRY_ADD_EXECUTOR_ROLE)) + assert.isFalse(await acl.hasPermission(scriptsRegistryFactory.address, scriptsRegistry.address, REGISTRY_ADD_EXECUTOR_ROLE)) + }) + } + + const itDoesNotCreateAnEVMScriptsRegistry = () => { + it('does not deploy an EVM script registry with a script executor', async () => { + assert.equal(await dao.getApp(APP_ADDR_NAMESPACE, EVM_SCRIPT_REGISTRY_APP_ID), ZERO_ADDRES) + assert.equal(await dao.getApp(APP_BASES_NAMESPACE, EVM_SCRIPT_REGISTRY_APP_ID), ZERO_ADDRES) + }) + } + + describe('newDAO', () => { + context('when it was created with an EVM scripts registry factory', () => { + before('create factory with an EVM scripts registry factory', async () => { + daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, scriptsRegistryFactory.address) + }) + + before('create a DAO', async () => { + receipt = await daoFactory.newDAO(root) + dao = Kernel.at(getEventArgument(receipt, 'DeployDAO', 'dao')) + acl = ACL.at(await dao.acl()) + }) + + itCreatesADao() + itDoesCreateAnEVMScriptsRegistry() + }) + + context('when it was created without an EVM scripts registry factory', () => { + before('create factory without an EVM scripts registry factory', async () => { + daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, ZERO_ADDRES) + }) + + before('create a DAO', async () => { + receipt = await daoFactory.newDAO(root) + dao = Kernel.at(getEventArgument(receipt, 'DeployDAO', 'dao')) + acl = ACL.at(await dao.acl()) + }) + + itCreatesADao() + itDoesNotCreateAnEVMScriptsRegistry() + }) + }) +}) From d10d195387269e8d51d0aa3e608ec44ecb9b8173 Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Thu, 9 May 2019 09:34:09 -0300 Subject: [PATCH 06/10] Tests: Refactors and enhancements (#525) * tests: refactors and enhancements * helpers: split assert and getter events files --- test/contracts/apm/apm_namehash.js | 20 +- test/contracts/apm/apm_registry.js | 76 +++----- test/contracts/apm/apm_repo.js | 18 +- test/contracts/apps/app_acl.js | 16 +- test/contracts/apps/app_base_lifecycle.js | 14 +- test/contracts/apps/app_funds.js | 14 +- test/contracts/apps/app_proxy.js | 44 ++--- test/contracts/apps/recovery_to_vault.js | 65 +++---- test/contracts/common/keccak_constants.js | 6 +- test/contracts/common/lifecycle.js | 10 +- test/contracts/common/reentrancy_guard.js | 10 +- test/contracts/common/safe_erc20.js | 11 +- test/contracts/common/time_helpers.js | 2 +- test/contracts/common/uint256_helpers.js | 8 +- test/contracts/common/unstructured_storage.js | 4 +- test/contracts/ens/ens_subdomains.js | 51 ++--- test/contracts/evmscript/evm_script.js | 70 +++---- test/contracts/factory/evm_script_factory.js | 25 +-- test/contracts/kernel/kernel_acl.js | 176 ++++++------------ test/contracts/kernel/kernel_apps.js | 62 +++--- test/contracts/kernel/kernel_funds.js | 23 +-- test/contracts/kernel/kernel_lifecycle.js | 56 +++--- test/contracts/kernel/kernel_upgrade.js | 20 +- test/contracts/lib/math/lib_safemath64.js | 32 ++-- test/contracts/lib/math/lib_safemath8.js | 32 ++-- test/helpers/assertEvent.js | 31 ++- test/helpers/events.js | 11 ++ 27 files changed, 345 insertions(+), 562 deletions(-) create mode 100644 test/helpers/events.js diff --git a/test/contracts/apm/apm_namehash.js b/test/contracts/apm/apm_namehash.js index df09813f5..f34d55ac5 100644 --- a/test/contracts/apm/apm_namehash.js +++ b/test/contracts/apm/apm_namehash.js @@ -1,32 +1,28 @@ -const namehash = require('eth-ens-namehash').hash +const { hash } = require('eth-ens-namehash') const APMNamehashMock = artifacts.require('APMNamehashMock') -contract('APM Name Hash', accounts => { +contract('APM Name Hash', () => { let apmNamehashMock before(async() => { - //console.log("eth: " + namehash('eth')) - //console.log("aragonpm.eth: " + namehash('aragonpm.eth')) apmNamehashMock = await APMNamehashMock.new() }) - const checkName = async (name) => { - const node = namehash(name + '.aragonpm.eth') - //await apmNamehashMock.getAPMNamehash(name) + const assertNamehash = async (name) => { + const node = hash(name + '.aragonpm.eth') const apmNamehash = await apmNamehashMock.getAPMNamehash(name) - //console.log("node: " + node) - return apmNamehash.toString() == node + assert.equal(node, apmNamehash.toString(), 'hashes do not match') } it('Kernel name hash matches', async () => { - assert.isTrue(await checkName('kernel'), 'hashes should match') + await assertNamehash('kernel') }) it('ACL name hash matches', async () => { - assert.isTrue(await checkName('acl'), 'hashes should match') + await assertNamehash('acl') }) it('EVM Registry name hash matches', async () => { - assert.isTrue(await checkName('evmreg'), 'hashes should match') + await assertNamehash('evmreg') }) }) diff --git a/test/contracts/apm/apm_registry.js b/test/contracts/apm/apm_registry.js index 876a5ec8d..79e84af5a 100644 --- a/test/contracts/apm/apm_registry.js +++ b/test/contracts/apm/apm_registry.js @@ -1,6 +1,7 @@ +const { hash } = require('eth-ens-namehash') +const { keccak_256 } = require('js-sha3') const { assertRevert } = require('../../helpers/assertThrow') -const namehash = require('eth-ens-namehash').hash -const keccak256 = require('js-sha3').keccak_256 +const { getEventArgument } = require('../../helpers/events') const ENS = artifacts.require('ENS') const ENSFactory = artifacts.require('ENSFactory') @@ -14,23 +15,16 @@ const APMRegistry = artifacts.require('APMRegistry') const ENSSubdomainRegistrar = artifacts.require('ENSSubdomainRegistrar') const Repo = artifacts.require('Repo') const APMRegistryFactory = artifacts.require('APMRegistryFactory') - const APMRegistryFactoryMock = artifacts.require('APMRegistryFactoryMock') -const getRepoFromLog = receipt => receipt.logs.filter(x => x.event == 'NewRepo')[0].args.repo - const EMPTY_BYTES = '0x' const ZERO_ADDR = '0x0000000000000000000000000000000000000000' -contract('APMRegistry', accounts => { +contract('APMRegistry', ([ensOwner, apmOwner, repoDev, notOwner, someone]) => { let baseDeployed, baseAddrs, ensFactory, apmFactory, daoFactory, ens, registry, acl - const ensOwner = accounts[0] - const apmOwner = accounts[1] - const repoDev = accounts[2] - const notOwner = accounts[5] - const rootNode = namehash('aragonpm.eth') - const testNode = namehash('test.aragonpm.eth') + const rootNode = hash('aragonpm.eth') + const testNode = hash('test.aragonpm.eth') before(async () => { const bases = [APMRegistry, Repo, ENSSubdomainRegistrar] @@ -48,8 +42,8 @@ contract('APMRegistry', accounts => { apmFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ZERO_ADDR, ensFactory.address) ens = ENS.at(await apmFactory.ens()) - const receipt = await apmFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner) - const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm + const receipt = await apmFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner) + const apmAddr = getEventArgument(receipt, 'DeployAPM', 'apm') registry = APMRegistry.at(apmAddr) const dao = Kernel.at(await registry.kernel()) @@ -61,13 +55,13 @@ contract('APMRegistry', accounts => { }) it('inits with existing ENS deployment', async () => { - const receipt = await ensFactory.newENS(accounts[0]) - const ens2 = ENS.at(receipt.logs.filter(l => l.event == 'DeployENS')[0].args.ens) + const receipt = await ensFactory.newENS(ensOwner) + const ens2 = ENS.at(getEventArgument(receipt, 'DeployENS', 'ens')) const newFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ens2.address, ZERO_ADDR) - await ens2.setSubnodeOwner(namehash('eth'), '0x'+keccak256('aragonpm'), newFactory.address) - const receipt2 = await newFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner) - const apmAddr = receipt2.logs.filter(l => l.event == 'DeployAPM')[0].args.apm + await ens2.setSubnodeOwner(hash('eth'), '0x'+keccak_256('aragonpm'), newFactory.address) + const receipt2 = await newFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner) + const apmAddr = getEventArgument(receipt2, 'DeployAPM', 'apm') const resolver = PublicResolver.at(await ens2.resolver(rootNode)) assert.equal(await resolver.addr(rootNode), apmAddr, 'rootnode should be resolve') @@ -85,7 +79,7 @@ contract('APMRegistry', accounts => { it('can create repo with version and dev can create new versions', async () => { const receipt = await registry.newRepoWithVersion('test', repoDev, [1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: apmOwner }) - const repo = Repo.at(getRepoFromLog(receipt)) + const repo = Repo.at(getEventArgument(receipt, 'NewRepo', 'repo')) assert.equal(await repo.getVersionsCount(), 1, 'should have created version') @@ -95,26 +89,20 @@ contract('APMRegistry', accounts => { }) it('fails to init with existing ENS deployment if not owner of tld', async () => { - const ensReceipt = await ensFactory.newENS(accounts[0]) - const ens2 = ENS.at(ensReceipt.logs.filter(l => l.event == 'DeployENS')[0].args.ens) + const ensReceipt = await ensFactory.newENS(ensOwner) + const ens2 = ENS.at(getEventArgument(ensReceipt, 'DeployENS', 'ens')) const newFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ens2.address, ZERO_ADDR) // Factory doesn't have ownership over 'eth' tld - await assertRevert(async () => { - await newFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner) - }) + await assertRevert(newFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner)) }) it('fails to create empty repo name', async () => { - return assertRevert(async () => { - await registry.newRepo('', repoDev, { from: apmOwner }) - }) + await assertRevert(registry.newRepo('', repoDev, { from: apmOwner })) }) it('fails to create repo if not in ACL', async () => { - return assertRevert(async () => { - await registry.newRepo('test', repoDev, { from: notOwner }) - }) + await assertRevert(registry.newRepo('test', repoDev, { from: notOwner })) }) context('> Creating test.aragonpm.eth repo', () => { @@ -122,11 +110,11 @@ contract('APMRegistry', accounts => { beforeEach(async () => { const receipt = await registry.newRepo('test', repoDev, { from: apmOwner }) - repo = Repo.at(getRepoFromLog(receipt)) + repo = Repo.at(getEventArgument(receipt, 'NewRepo', 'repo')) }) it('resolver is setup correctly', async () => { - const resolverNode = namehash('resolver.eth') + const resolverNode = hash('resolver.eth') const publicResolver = PublicResolver.at(await ens.resolver(resolverNode)) assert.equal(await ens.resolver(testNode), await publicResolver.addr(resolverNode), 'resolver should be set to public resolver') @@ -138,9 +126,7 @@ contract('APMRegistry', accounts => { }) it('fails when creating repo with existing name', async () => { - return assertRevert(async () => { - await registry.newRepo('test', repoDev) - }) + await assertRevert(registry.newRepo('test', repoDev)) }) it('repo dev can create versions', async () => { @@ -152,7 +138,7 @@ contract('APMRegistry', accounts => { it('repo dev can authorize someone to interact with repo', async () => { await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev }) - const newOwner = accounts[8] + const newOwner = someone await acl.grantPermission(newOwner, repo.address, await repo.CREATE_VERSION_ROLE(), { from: repoDev }) @@ -166,15 +152,11 @@ contract('APMRegistry', accounts => { await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev }) await acl.revokePermission(repoDev, repo.address, await repo.CREATE_VERSION_ROLE(), { from: repoDev }) - return assertRevert(async () => { - await repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev }) - }) + await assertRevert(repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })) }) it('cannot create versions if not in ACL', async () => { - return assertRevert(async () => { - await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: notOwner }) - }) + await assertRevert(repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: notOwner })) }) }) @@ -186,15 +168,11 @@ contract('APMRegistry', accounts => { }) it('fails if factory doesnt give permission to create names', async () => { - await assertRevert(async () => { - await apmFactoryMock.newFailingAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, true) - }) + await assertRevert(apmFactoryMock.newFailingAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner, true)) }) it('fails if factory doesnt give permission to create permissions', async () => { - await assertRevert(async () => { - await apmFactoryMock.newFailingAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, false) - }) + await assertRevert(apmFactoryMock.newFailingAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner, false)) }) }) }) diff --git a/test/contracts/apm/apm_repo.js b/test/contracts/apm/apm_repo.js index 900023fed..60bf6c66f 100644 --- a/test/contracts/apm/apm_repo.js +++ b/test/contracts/apm/apm_repo.js @@ -10,7 +10,7 @@ contract('Repo', accounts => { beforeEach(async () => { repo = await Repo.new() - await repo.initialize(); + await repo.initialize() }) it('computes correct valid bumps', async () => { @@ -32,9 +32,7 @@ contract('Repo', accounts => { // valid version as being a correct bump from 0.0.0 it('cannot create invalid first version', async () => { - return assertRevert(async () => { - await repo.newVersion([1, 1, 0], ZERO_ADDR, EMPTY_BYTES) - }) + await assertRevert(repo.newVersion([1, 1, 0], ZERO_ADDR, EMPTY_BYTES)) }) context('creating initial version', () => { @@ -78,21 +76,15 @@ contract('Repo', accounts => { }) it('fails when changing contract address in non major version', async () => { - return assertRevert(async () => { - await repo.newVersion([1, 1, 0], accounts[2], initialContent) - }) + await assertRevert(repo.newVersion([1, 1, 0], accounts[2], initialContent)) }) it('fails when version bump is invalid', async () => { - return assertRevert(async () => { - await repo.newVersion([1, 2, 0], initialCode, initialContent) - }) + await assertRevert(repo.newVersion([1, 2, 0], initialCode, initialContent)) }) it('fails if requesting version 0', async () => { - return assertRevert(async () => { - await repo.getByVersionId(0) - }) + await assertRevert(repo.getByVersionId(0)) }) context('adding new version', () => { diff --git a/test/contracts/apps/app_acl.js b/test/contracts/apps/app_acl.js index cd0549f02..ca7e5467e 100644 --- a/test/contracts/apps/app_acl.js +++ b/test/contracts/apps/app_acl.js @@ -1,6 +1,6 @@ -const { assertRevert } = require('../../helpers/assertThrow') -const { onlyIf } = require('../../helpers/onlyIf') const { hash } = require('eth-ens-namehash') +const { onlyIf } = require('../../helpers/onlyIf') +const { assertRevert } = require('../../helpers/assertThrow') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') @@ -95,9 +95,7 @@ contract('App ACL', accounts => { }) it('fails when called by unauthorized entity', async () => { - return assertRevert(async () => { - await app.setValue(10, { from: unauthorized }) - }) + await assertRevert(app.setValue(10, { from: unauthorized })) }) onlyAppProxyUpgradeable(() => @@ -106,9 +104,7 @@ contract('App ACL', accounts => { const appProxy = await AppProxyUpgradeable.new(kernel.address, unknownId, EMPTY_BYTES) const app = AppStub.at(appProxy.address) - return assertRevert(async () => { - await app.setValue(10) - }) + await assertRevert(app.setValue(10)) }) ) @@ -138,9 +134,7 @@ contract('App ACL', accounts => { }) it('parametrized app call fails if param eval fails', async () => { - return assertRevert(async () => { - await app.setValueParam(failValue, { from: paramsGrantee }) - }) + await assertRevert(app.setValueParam(failValue, { from: paramsGrantee })) }) }) }) diff --git a/test/contracts/apps/app_base_lifecycle.js b/test/contracts/apps/app_base_lifecycle.js index 94ee80438..b9bf5fecc 100644 --- a/test/contracts/apps/app_base_lifecycle.js +++ b/test/contracts/apps/app_base_lifecycle.js @@ -1,25 +1,21 @@ -const { assertRevert } = require('../../helpers/assertThrow') -const { getBlockNumber } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') const { soliditySha3 } = require('web3-utils') +const { getBlockNumber } = require('../../helpers/web3') +const { assertRevert } = require('../../helpers/assertThrow') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') const KernelProxy = artifacts.require('KernelProxy') - const AragonApp = artifacts.require('AragonApp') -const AppProxyUpgradeable = artifacts.require('AppProxyUpgradeable') // Mocks const UnsafeAragonAppMock = artifacts.require('UnsafeAragonAppMock') -const APP_ID = hash('app.aragonpm.test') const FAKE_ROLE = soliditySha3('FAKE_ROLE') const ZERO_ADDR = '0x0000000000000000000000000000000000000000' -contract('App base lifecycle', accounts => { +contract('App base lifecycle', ([permissionsRoot]) => { let aclBase, kernelBase - const permissionsRoot = accounts[0] before(async () => { kernelBase = await Kernel.new(true) // petrify immediately @@ -88,9 +84,7 @@ contract('App base lifecycle', accounts => { }) it('throws on reinitialization', async () => { - return assertRevert(async () => { - await app.initialize() - }) + await assertRevert(app.initialize()) }) it('should still not be usable without a kernel', async () => { diff --git a/test/contracts/apps/app_funds.js b/test/contracts/apps/app_funds.js index b001d9ea4..623736272 100644 --- a/test/contracts/apps/app_funds.js +++ b/test/contracts/apps/app_funds.js @@ -1,7 +1,7 @@ const { hash } = require('eth-ens-namehash') -const { assertRevert } = require('../../helpers/assertThrow') -const { getBalance } = require('../../helpers/web3') 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') @@ -19,12 +19,10 @@ const APP_ID = hash('stub.aragonpm.test') const EMPTY_BYTES = '0x' const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies -contract('App funds', accounts => { +contract('App funds', ([permissionsRoot]) => { let aclBase, kernelBase let APP_BASES_NAMESPACE - const permissionsRoot = accounts[0] - before(async () => { kernelBase = await Kernel.new(true) // petrify immediately aclBase = await ACL.new() @@ -84,15 +82,13 @@ contract('App funds', accounts => { app = appBaseType.at(appProxy.address) } - await app.initialize(); + await app.initialize() }) it('cannot receive ETH', async () => { assert.isTrue(await app.hasInitialized(), 'should have been initialized') - await assertRevert(async () => { - await app.sendTransaction({ value: 1, gas: SEND_ETH_GAS }) - }) + await assertRevert(app.sendTransaction({ value: 1, gas: SEND_ETH_GAS })) }) onlyAppStubDepositable(() => { diff --git a/test/contracts/apps/app_proxy.js b/test/contracts/apps/app_proxy.js index 2e3ea991c..c8862e7ca 100644 --- a/test/contracts/apps/app_proxy.js +++ b/test/contracts/apps/app_proxy.js @@ -1,7 +1,7 @@ -const { assertRevert } = require('../../helpers/assertThrow') +const { hash } = require('eth-ens-namehash') const { onlyIf } = require('../../helpers/onlyIf') const { getBlockNumber } = require('../../helpers/web3') -const { hash } = require('eth-ens-namehash') +const { assertRevert } = require('../../helpers/assertThrow') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') @@ -19,13 +19,11 @@ const APP_ID = hash('stub.aragonpm.test') const ZERO_ADDR = '0x0000000000000000000000000000000000000000' const EMPTY_BYTES = '0x' -contract('App proxy', accounts => { +contract('App proxy', ([permissionsRoot]) => { let aclBase, appBase1, appBase2, kernelBase, acl, kernel let APP_BASES_NAMESPACE, APP_ROLE let UPGRADEABLE, FORWARDING - const permissionsRoot = accounts[0] - // Initial setup before(async () => { appBase1 = await AppStub.new() @@ -103,9 +101,7 @@ contract('App proxy', accounts => { it('fails if initializing on constructor before setting app code', async () => { const initializationPayload = appBase1.contract.initialize.getData() - return assertRevert(async () => { - await AppProxyUpgradeable.new(kernel.address, APP_ID, initializationPayload) - }) + await assertRevert(AppProxyUpgradeable.new(kernel.address, APP_ID, initializationPayload)) }) }) @@ -113,32 +109,24 @@ contract('App proxy', accounts => { const FAKE_APP_ID = hash('fake.aragonpm.test') it("fails if code hasn't been set yet", async () => { - await assertRevert(async () => { - await AppProxyPinned.new(kernel.address, FAKE_APP_ID, EMPTY_BYTES) - }) + await assertRevert(AppProxyPinned.new(kernel.address, FAKE_APP_ID, EMPTY_BYTES)) }) it("fails if code set isn't a contract", async () => { const kernelMock = await KernelSetAppMock.new() await kernelMock.setApp(APP_BASES_NAMESPACE, FAKE_APP_ID, '0x1234') - await assertRevert(async () => { - await AppProxyPinned.new(kernelMock.address, FAKE_APP_ID, EMPTY_BYTES) - }) + await assertRevert(AppProxyPinned.new(kernelMock.address, FAKE_APP_ID, EMPTY_BYTES)) }) }) context('> Fails on bad kernel', () => { it('fails if kernel address is 0', async () => { - return assertRevert(async () => { - await appProxyContract.new(ZERO_ADDR, APP_ID, EMPTY_BYTES) - }) + await assertRevert(appProxyContract.new(ZERO_ADDR, APP_ID, EMPTY_BYTES)) }) it('fails if kernel address is not a contract', async () => { - return assertRevert(async () => { - await appProxyContract.new('0x1234', APP_ID, EMPTY_BYTES) - }) + await assertRevert(appProxyContract.new('0x1234', APP_ID, EMPTY_BYTES)) }) }) @@ -167,16 +155,12 @@ contract('App proxy', accounts => { }) it('cannot reinitialize', async () => { - return assertRevert(async () => { - await app.initialize() - }) + await assertRevert(app.initialize()) }) it('fails if init fails', async () => { const badInit = '0x1234' - return assertRevert(async () => { - await appProxyContract.new(kernel.address, APP_ID, badInit) - }) + await assertRevert(appProxyContract.new(kernel.address, APP_ID, badInit)) }) it('should return values correctly', async () => { @@ -208,9 +192,7 @@ contract('App proxy', accounts => { }) it("fails calling functionality requiring initialization (if it's not)", async () => { - return assertRevert(async () => { - await app.requiresInitialization() - }) + await assertRevert(app.requiresInitialization()) }) it('allows calling functionality requiring initialization after initializing', async () => { @@ -246,9 +228,7 @@ contract('App proxy', accounts => { await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase2.address) // The upgraded app (AppStub2) doesn't have `setValue()` anymore - return assertRevert(async () => { - await app.setValue(10) - }) + await assertRevert(app.setValue(10)) }) }) diff --git a/test/contracts/apps/recovery_to_vault.js b/test/contracts/apps/recovery_to_vault.js index 76dd7ddb9..6f5e2010c 100644 --- a/test/contracts/apps/recovery_to_vault.js +++ b/test/contracts/apps/recovery_to_vault.js @@ -1,14 +1,13 @@ -const assertEvent = require('../../helpers/assertEvent') -const { assertRevert } = require('../../helpers/assertThrow') -const { skipCoverage } = require('../../helpers/coverage') -const { getBalance } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') +const { getBalance } = require('../../helpers/web3') +const { skipCoverage } = require('../../helpers/coverage') +const { assertRevert } = require('../../helpers/assertThrow') +const { getNewProxyAddress } = require('../../helpers/events') +const { assertAmountOfEvents, assertEvent } = require('../../helpers/assertEvent')(web3) -const DAOFactory = artifacts.require('DAOFactory') +const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') const KernelProxy = artifacts.require('KernelProxy') -const ACL = artifacts.require('ACL') -const AppProxyUpgradeable = artifacts.require('AppProxyUpgradeable') // Mocks const AppStubDepositable = artifacts.require('AppStubDepositable') @@ -20,24 +19,20 @@ const TokenReturnMissingMock = artifacts.require('TokenReturnMissingMock') const VaultMock = artifacts.require('VaultMock') const KernelDepositableMock = artifacts.require('KernelDepositableMock') -const getEvent = (receipt, event, arg) => { return receipt.logs.filter(l => l.event == event)[0].args[arg] } - const APP_ID = hash('stub.aragonpm.test') const EMPTY_BYTES = '0x' const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies -contract('Recovery to vault', accounts => { +contract('Recovery to vault', ([permissionsRoot]) => { let aclBase, appBase, appConditionalRecoveryBase let APP_ADDR_NAMESPACE, ETH - const permissionsRoot = accounts[0] - // Helpers const recoverEth = async ({ shouldFail, target, vault }) => { const amount = 1 const initialBalance = await getBalance(target.address) const initialVaultBalance = await getBalance(vault.address) - const r = await target.sendTransaction({ value: 1, gas: SEND_ETH_GAS }) + await target.sendTransaction({ value: 1, gas: SEND_ETH_GAS }) assert.equal((await getBalance(target.address)).valueOf(), initialBalance.plus(amount), 'Target initial balance should be correct') const recoverAction = () => target.transferToVault(ETH) @@ -51,16 +46,14 @@ contract('Recovery to vault', accounts => { assert.equal((await getBalance(target.address)).valueOf(), 0, 'Target balance should be 0') assert.equal((await getBalance(vault.address)).valueOf(), initialVaultBalance.plus(initialBalance).plus(amount), 'Vault balance should include recovered amount') - assert.equal(getEvent(recoverReceipt, 'RecoverToVault', 'vault'), vault.address, 'RecoverToVault event should have correct vault') - assert.equal(getEvent(recoverReceipt, 'RecoverToVault', 'token'), ETH, 'RecoverToVault event should have correct token') - assert.equal(getEvent(recoverReceipt, 'RecoverToVault', 'amount'), amount, 'RecoverToVault event should have correct amount') - assertEvent(recoverReceipt, 'RecoverToVault', 1) + assertAmountOfEvents(recoverReceipt, 'RecoverToVault') + assertEvent(recoverReceipt, 'RecoverToVault', { vault: vault.address, token: ETH, amount: amount }) } } const recoverTokens = async ({ shouldFail, tokenContract, target, vault }) => { const amount = 1 - const token = await tokenContract.new(accounts[0], 1000) + const token = await tokenContract.new(permissionsRoot, 1000) const initialBalance = await token.balanceOf(target.address) const initialVaultBalance = await token.balanceOf(vault.address) await token.transfer(target.address, amount) @@ -77,16 +70,14 @@ contract('Recovery to vault', accounts => { assert.equal((await token.balanceOf(target.address)).valueOf(), 0, 'Target balance should be 0') assert.equal((await token.balanceOf(vault.address)).valueOf(), initialVaultBalance.plus(initialBalance).plus(amount), 'Vault balance should include recovered amount') - assert.equal(getEvent(recoverReceipt, 'RecoverToVault', 'vault'), vault.address, 'RecoverToVault event should have correct vault') - assert.equal(getEvent(recoverReceipt, 'RecoverToVault', 'token'), token.address, 'RecoverToVault event should have correct token') - assert.equal(getEvent(recoverReceipt, 'RecoverToVault', 'amount'), amount, 'RecoverToVault event should have correct amount') - assertEvent(recoverReceipt, 'RecoverToVault', 1) + assertAmountOfEvents(recoverReceipt, 'RecoverToVault') + assertEvent(recoverReceipt, 'RecoverToVault', { vault: vault.address, token: token.address, amount: amount }) } } const failingRecoverTokens = async ({ tokenContract, target, vault }) => { const amount = 1 - const token = await tokenContract.new(accounts[0], 1000) + const token = await tokenContract.new(permissionsRoot, 1000) const initialBalance = await token.balanceOf(target.address) const initialVaultBalance = await token.balanceOf(vault.address) await token.transfer(target.address, amount) @@ -96,7 +87,7 @@ contract('Recovery to vault', accounts => { await token.setAllowTransfer(false) // Try to transfer - await assertRevert(() => target.transferToVault(token.address)) + await assertRevert(target.transferToVault(token.address)) assert.equal((await token.balanceOf(target.address)).valueOf(), initialBalance.plus(amount), 'Target balance should be same as before') assert.equal((await token.balanceOf(vault.address)).valueOf(), initialVaultBalance, 'Vault balance should should be same as before') @@ -109,9 +100,7 @@ contract('Recovery to vault', accounts => { await kernel.setRecoveryVaultAppId(vaultId) const r = await target.sendTransaction({ value: 1, gas: SEND_ETH_GAS }) assert.equal((await getBalance(target.address)).valueOf(), initialBalance.plus(amount), 'Target initial balance should be correct') - return assertRevert(async () => { - await target.transferToVault(ETH) - }) + await assertRevert(target.transferToVault(ETH)) } // Token test groups @@ -163,7 +152,7 @@ contract('Recovery to vault', accounts => { kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address) } - await kernel.initialize(aclBase.address, permissionsRoot); + await kernel.initialize(aclBase.address, permissionsRoot) const acl = ACL.at(await kernel.acl()) const r = await kernel.APP_MANAGER_ROLE() await acl.createPermission(permissionsRoot, kernel.address, r, permissionsRoot) @@ -192,7 +181,7 @@ contract('Recovery to vault', accounts => { } else if (vaultType === 'VaultProxy') { // This doesn't automatically setup the recovery address const receipt = await kernel.newAppInstance(vaultId, vaultBase.address, EMPTY_BYTES, false) - const vaultProxyAddress = getEvent(receipt, 'NewAppProxy', 'proxy') + const vaultProxyAddress = getNewProxyAddress(receipt) vault = VaultMock.at(vaultProxyAddress) } await vault.initialize() @@ -202,9 +191,7 @@ contract('Recovery to vault', accounts => { }) it('kernel cannot receive ETH', async () => - await assertRevert( - () => kernel.sendTransaction({ value: 1, gas: 31000 }) - ) + await assertRevert(kernel.sendTransaction({ value: 1, gas: 31000 })) ) for (const { title, tokenContract } of tokenTestGroups) { @@ -249,7 +236,7 @@ contract('Recovery to vault', accounts => { beforeEach(async () => { // Setup app const receipt = await kernel.newAppInstance(APP_ID, appBase.address, EMPTY_BYTES, false) - const appProxy = getEvent(receipt, 'NewAppProxy', 'proxy') + const appProxy = getNewProxyAddress(receipt) const app = AppStubDepositable.at(appProxy) await app.enableDeposits() @@ -257,15 +244,11 @@ contract('Recovery to vault', accounts => { }) it('cannot send 0 ETH to proxy', async () => { - await assertRevert(async () => { - await target.sendTransaction({ value: 0, gas: SEND_ETH_GAS }) - }) + await assertRevert(target.sendTransaction({ value: 0, gas: SEND_ETH_GAS })) }) it('cannot send ETH with data to proxy', async () => { - await assertRevert(async () => { - await target.sendTransaction({ value: 1, data: '0x01', gas: SEND_ETH_GAS }) - }) + await assertRevert(target.sendTransaction({ value: 1, data: '0x01', gas: SEND_ETH_GAS })) }) it('recovers ETH', skipCoverageIfVaultProxy(async () => @@ -299,7 +282,7 @@ contract('Recovery to vault', accounts => { beforeEach(async () => { // Setup app with conditional recovery code const receipt = await kernel.newAppInstance(APP_ID, appConditionalRecoveryBase.address, EMPTY_BYTES, false) - const appProxy = getEvent(receipt, 'NewAppProxy', 'proxy') + const appProxy = getNewProxyAddress(receipt) const app = AppStubConditionalRecovery.at(appProxy) await app.initialize() @@ -354,7 +337,7 @@ contract('Recovery to vault', accounts => { const vaultId = hash('vault.aragonpm.test') const vaultBase = await VaultMock.new() const vaultReceipt = await kernel.newAppInstance(vaultId, vaultBase.address, EMPTY_BYTES, true) - const vaultAddress = getEvent(vaultReceipt, 'NewAppProxy', 'proxy') + const vaultAddress = getNewProxyAddress(vaultReceipt) vault = VaultMock.at(vaultAddress) await vault.initialize() diff --git a/test/contracts/common/keccak_constants.js b/test/contracts/common/keccak_constants.js index c6567bf6d..972bb9b03 100644 --- a/test/contracts/common/keccak_constants.js +++ b/test/contracts/common/keccak_constants.js @@ -1,10 +1,6 @@ -const namehash = require('eth-ens-namehash').hash -const keccak_256 = require('js-sha3').keccak_256 - const getContract = name => artifacts.require(name) -const keccak256 = (name) => '0x' + keccak_256(name) -contract('Constants', accounts => { +contract('Constants', () => { let keccakConstants before(async () => { diff --git a/test/contracts/common/lifecycle.js b/test/contracts/common/lifecycle.js index cabe2f6f7..ee0d11d6e 100644 --- a/test/contracts/common/lifecycle.js +++ b/test/contracts/common/lifecycle.js @@ -1,11 +1,11 @@ +const reverts = require('../../helpers/revertStrings') const { assertRevert } = require('../../helpers/assertThrow') const { getBlockNumber } = require('../../helpers/web3') -const reverts = require('../../helpers/revertStrings') // Mocks const LifecycleMock = artifacts.require('LifecycleMock') -contract('Lifecycle', accounts => { +contract('Lifecycle', () => { let lifecycle beforeEach(async () => { @@ -38,11 +38,11 @@ contract('Lifecycle', accounts => { }) it('cannot be re-initialized', async () => { - assertRevert(lifecycle.initializeMock(), reverts.INIT_ALREADY_INITIALIZED) + await assertRevert(lifecycle.initializeMock(), reverts.INIT_ALREADY_INITIALIZED) }) it('cannot be petrified', async () => { - assertRevert(lifecycle.petrifyMock(), reverts.INIT_ALREADY_INITIALIZED) + await assertRevert(lifecycle.petrifyMock(), reverts.INIT_ALREADY_INITIALIZED) }) }) @@ -60,7 +60,7 @@ contract('Lifecycle', accounts => { }) it('cannot be petrified again', async () => { - assertRevert(lifecycle.petrifyMock(), reverts.INIT_ALREADY_INITIALIZED) + await assertRevert(lifecycle.petrifyMock(), reverts.INIT_ALREADY_INITIALIZED) }) it('has initialization block in the future', async () => { diff --git a/test/contracts/common/reentrancy_guard.js b/test/contracts/common/reentrancy_guard.js index 2e9944092..6a3c4c8ec 100644 --- a/test/contracts/common/reentrancy_guard.js +++ b/test/contracts/common/reentrancy_guard.js @@ -2,7 +2,7 @@ const { assertRevert } = require('../../helpers/assertThrow') const ZERO_ADDR = '0x0000000000000000000000000000000000000000' -contract('ReentrancyGuard', accounts => { +contract('ReentrancyGuard', () => { let reentrancyMock async function assertReentrancyMutex(mutex, msg) { @@ -49,9 +49,7 @@ contract('ReentrancyGuard', accounts => { }) it('can not call non-re-entrant function if re-entrancy mutex is enabled', async () => { - await assertRevert(async () => { - await reentrancyMock.nonReentrantCall(ZERO_ADDR) - }) + await assertRevert(reentrancyMock.nonReentrantCall(ZERO_ADDR)) assert.equal((await reentrancyMock.callCounter()).toString(), 0, 'should not have called') }) }) @@ -85,9 +83,7 @@ contract('ReentrancyGuard', accounts => { }) it('disallows re-entering non-re-entrant call', async () => { - await assertRevert(async () => { - await reentrancyMock.nonReentrantCall(reentrantActor.address) - }) + await assertRevert(reentrancyMock.nonReentrantCall(reentrantActor.address)) assert.equal((await reentrancyMock.callCounter()).toString(), 0, 'should not have completed any calls') }) diff --git a/test/contracts/common/safe_erc20.js b/test/contracts/common/safe_erc20.js index b92bfe5e5..00c08a311 100644 --- a/test/contracts/common/safe_erc20.js +++ b/test/contracts/common/safe_erc20.js @@ -1,4 +1,4 @@ -const { assertRevert } = require('../../helpers/assertThrow') +const { getEventArgument } = require('../../helpers/events') // Mocks const SafeERC20Mock = artifacts.require('SafeERC20Mock') @@ -6,14 +6,9 @@ const TokenMock = artifacts.require('TokenMock') const TokenReturnFalseMock = artifacts.require('TokenReturnFalseMock') const TokenReturnMissingMock = artifacts.require('TokenReturnMissingMock') -const assertMockResult = (receipt, result) => { - const events = receipt.logs.filter(x => x.event == 'Result') - const eventArg = events[0].args.result - assert.equal(eventArg, result, `Result not expected (got ${eventArg} instead of ${result})`) -} +const assertMockResult = (receipt, result) => assert.equal(getEventArgument(receipt, 'Result', 'result'), result, `result does not match`) -contract('SafeERC20', accounts => { - const [owner, receiver] = accounts +contract('SafeERC20', ([owner, receiver]) => { const initialBalance = 10000 let safeERC20Mock diff --git a/test/contracts/common/time_helpers.js b/test/contracts/common/time_helpers.js index 599afe88b..13530fd32 100644 --- a/test/contracts/common/time_helpers.js +++ b/test/contracts/common/time_helpers.js @@ -1,4 +1,4 @@ -contract('TimeHelpers', accounts => { +contract('TimeHelpers', () => { let timeHelpersMock before(async () => { diff --git a/test/contracts/common/uint256_helpers.js b/test/contracts/common/uint256_helpers.js index 3173d957c..7be2a8ff4 100644 --- a/test/contracts/common/uint256_helpers.js +++ b/test/contracts/common/uint256_helpers.js @@ -1,6 +1,6 @@ const { assertRevert } = require('../../helpers/assertThrow') -contract('Uint256 Helpers test', accounts => { +contract('Uint256 Helpers test', () => { let uint256Mock before(async () => { @@ -9,13 +9,11 @@ contract('Uint256 Helpers test', accounts => { it('converts from uint256 to uint64', async () => { const a = 1234 - assert.equal((await uint256Mock.convert(a)).toString(), a, "Values should match") + assert.equal((await uint256Mock.convert(a)).toString(), a, 'values should match') }) it('fails converting from uint256 to uint64 if too big', async () => { const a = new web3.BigNumber(2).pow(64) - return assertRevert(async () => { - await uint256Mock.convert(a) - }) + await assertRevert(uint256Mock.convert(a)) }) }) diff --git a/test/contracts/common/unstructured_storage.js b/test/contracts/common/unstructured_storage.js index e102780ca..a5bff25c1 100644 --- a/test/contracts/common/unstructured_storage.js +++ b/test/contracts/common/unstructured_storage.js @@ -1,5 +1,5 @@ -const AppStub = artifacts.require('AppStub') const Kernel = artifacts.require('Kernel') +const AppStub = artifacts.require('AppStub') // Mocks const AppStorageMock = artifacts.require('AppStorageMock') @@ -9,7 +9,7 @@ const InitializableStorageMock = artifacts.require('InitializableStorageMock') const KernelPinnedStorageMock = artifacts.require('KernelPinnedStorageMock') const ReentrancyGuardMock = artifacts.require('ReentrancyGuardMock') -contract('Unstructured storage', accounts => { +contract('Unstructured storage', () => { context('> AppStorage', () => { let appStorage diff --git a/test/contracts/ens/ens_subdomains.js b/test/contracts/ens/ens_subdomains.js index f3168f684..d34b4640d 100644 --- a/test/contracts/ens/ens_subdomains.js +++ b/test/contracts/ens/ens_subdomains.js @@ -1,37 +1,30 @@ +const { hash } = require('eth-ens-namehash') +const { keccak_256 } = require('js-sha3') const { assertRevert } = require('../../helpers/assertThrow') -const namehash = require('eth-ens-namehash').hash -const keccak256 = require('js-sha3').keccak_256 +const { getEventArgument } = require('../../helpers/events') const ENS = artifacts.require('ENS') const ENSFactory = artifacts.require('ENSFactory') -const PublicResolver = artifacts.require('PublicResolver') -const Kernel = artifacts.require('Kernel') +const Repo = artifacts.require('Repo') const ACL = artifacts.require('ACL') - +const Kernel = artifacts.require('Kernel') +const DAOFactory = artifacts.require('DAOFactory') const APMRegistry = artifacts.require('APMRegistry') +const APMRegistryFactory = artifacts.require('APMRegistryFactory') const AppProxyUpgradeable = artifacts.require('AppProxyUpgradeable') const ENSSubdomainRegistrar = artifacts.require('ENSSubdomainRegistrar') -const Repo = artifacts.require('Repo') -const APMRegistryFactory = artifacts.require('APMRegistryFactory') -const DAOFactory = artifacts.require('DAOFactory') const EMPTY_BYTES = '0x' const ZERO_ADDR = '0x0000000000000000000000000000000000000000' // Using APMFactory in order to reuse it -contract('ENSSubdomainRegistrar', accounts => { +contract('ENSSubdomainRegistrar', ([_, apmOwner, notOwner]) => { let baseDeployed, apmFactory, ensFactory, dao, daoFactory, ens, registrar let APP_BASES_NAMESPACE - const ensOwner = accounts[0] - const apmOwner = accounts[1] - const repoDev = accounts[2] - const notOwner = accounts[5] - - const rootNode = namehash('aragonpm.eth') - const holanode = namehash('hola.aragonpm.eth') - const holalabel = '0x'+keccak256('hola') + const holanode = hash('hola.aragonpm.eth') + const holalabel = '0x'+keccak_256('hola') before(async () => { const bases = [APMRegistry, Repo, ENSSubdomainRegistrar] @@ -51,8 +44,8 @@ contract('ENSSubdomainRegistrar', accounts => { apmFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ZERO_ADDR, ensFactory.address) ens = ENS.at(await apmFactory.ens()) - const receipt = await apmFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner) - const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm + const receipt = await apmFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner) + const apmAddr = getEventArgument(receipt, 'DeployAPM', 'apm') const registry = APMRegistry.at(apmAddr) dao = Kernel.at(await registry.kernel()) @@ -68,26 +61,20 @@ contract('ENSSubdomainRegistrar', accounts => { it('can create name', async () => { await registrar.createName(holalabel, apmOwner, { from: apmOwner }) - assert.equal(await ens.owner(namehash('hola.aragonpm.eth')), apmOwner, 'should have created name') + assert.equal(await ens.owner(hash('hola.aragonpm.eth')), apmOwner, 'should have created name') }) it('fails if creating names twice', async () => { await registrar.createName(holalabel, apmOwner, { from: apmOwner }) - return assertRevert(async () => { - await registrar.createName(holalabel, apmOwner, { from: apmOwner }) - }) + await assertRevert(registrar.createName(holalabel, apmOwner, { from: apmOwner })) }) it('fails if deleting name not yet created', async () => { - return assertRevert(async () => { - await registrar.deleteName(holalabel, { from: apmOwner }) - }) + await assertRevert(registrar.deleteName(holalabel, { from: apmOwner })) }) it('fails if not authorized to create name', async () => { - return assertRevert(async () => { - await registrar.createName(holalabel, apmOwner, { from: notOwner }) - }) + await assertRevert(registrar.createName(holalabel, apmOwner, { from: notOwner })) }) it('can delete names', async () => { @@ -106,11 +93,9 @@ contract('ENSSubdomainRegistrar', accounts => { it('fails if initializing without rootnode ownership', async () => { const ens = await ENS.new() - const newRegProxy = await AppProxyUpgradeable.new(dao.address, namehash('apm-enssub.aragonpm.eth'), EMPTY_BYTES) + const newRegProxy = await AppProxyUpgradeable.new(dao.address, hash('apm-enssub.aragonpm.eth'), EMPTY_BYTES) const newReg = ENSSubdomainRegistrar.at(newRegProxy.address) - await assertRevert(async () => { - await newReg.initialize(ens.address, holanode) - }) + await assertRevert(newReg.initialize(ens.address, holanode)) }) }) diff --git a/test/contracts/evmscript/evm_script.js b/test/contracts/evmscript/evm_script.js index 8f919421e..3b36ae593 100644 --- a/test/contracts/evmscript/evm_script.js +++ b/test/contracts/evmscript/evm_script.js @@ -1,14 +1,14 @@ +const reverts = require('../../helpers/revertStrings') const { rawEncode } = require('ethereumjs-abi') const { soliditySha3 } = require('web3-utils') - -const assertEvent = require('../../helpers/assertEvent') const { assertRevert } = require('../../helpers/assertThrow') const { createExecutorId, encodeCallScript } = require('../../helpers/evmScript') -const reverts = require('../../helpers/revertStrings') +const { assertEvent, assertAmountOfEvents } = require('../../helpers/assertEvent')(web3) +const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') +const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') const KernelProxy = artifacts.require('KernelProxy') -const ACL = artifacts.require('ACL') const EVMScriptRegistry = artifacts.require('EVMScriptRegistry') const CallsScript = artifacts.require('CallsScript') const IEVMScriptExecutor = artifacts.require('IEVMScriptExecutor') @@ -24,16 +24,14 @@ const EVMScriptRegistryConstantsMock = artifacts.require('EVMScriptRegistryConst const ZERO_ADDR = '0x0000000000000000000000000000000000000000' const EMPTY_BYTES = '0x' -contract('EVM Script', accounts => { +contract('EVM Script', ([_, boss]) => { let kernelBase, aclBase, evmScriptRegBase, dao, acl, evmScriptReg let scriptExecutorMock, scriptExecutorNoReturnMock, scriptExecutorRevertMock let APP_BASES_NAMESPACE, APP_ADDR_NAMESPACE, APP_MANAGER_ROLE let EVMSCRIPT_REGISTRY_APP_ID, REGISTRY_ADD_EXECUTOR_ROLE, REGISTRY_MANAGER_ROLE let ERROR_MOCK_REVERT, ERROR_EXECUTION_TARGET - const boss = accounts[1] - - const scriptRunnerAppId = '0x1234' + const SCRIPT_RUNNER_APP_ID = '0x1234' before(async () => { kernelBase = await Kernel.new(true) // petrify immediately @@ -68,7 +66,7 @@ contract('EVM Script', accounts => { // Set up script registry (MUST use correct app ID and set as default app) const initPayload = evmScriptRegBase.contract.initialize.getData() const evmScriptRegReceipt = await dao.newAppInstance(EVMSCRIPT_REGISTRY_APP_ID, evmScriptRegBase.address, initPayload, true, { from: boss }) - evmScriptReg = EVMScriptRegistry.at(evmScriptRegReceipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + evmScriptReg = EVMScriptRegistry.at(getNewProxyAddress(evmScriptRegReceipt)) }) context('> Registry', () => { @@ -92,7 +90,7 @@ contract('EVM Script', accounts => { it('can add a new executor', async () => { const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorMock.address, { from: boss }) - const newExecutorId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + const newExecutorId = getEventArgument(receipt, 'EnableExecutor', 'executorId') const [executorAddress, executorEnabled] = await evmScriptReg.executors(newExecutorId) const newExecutor = IEVMScriptExecutor.at(executorAddress) @@ -110,8 +108,7 @@ contract('EVM Script', accounts => { beforeEach(async () => { const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorMock.address, { from: boss }) - - installedExecutorId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + installedExecutorId = getEventArgument(receipt, 'EnableExecutor', 'executorId') }) it('can disable an executor', async () => { @@ -123,7 +120,7 @@ contract('EVM Script', accounts => { const receipt = await evmScriptReg.disableScriptExecutor(installedExecutorId, { from: boss }) executorEntry = await evmScriptReg.executors(installedExecutorId) - assertEvent(receipt, 'DisableExecutor') + assertAmountOfEvents(receipt, 'DisableExecutor') assert.isFalse(executorEntry[1], "executor should now be disabled") assert.equal(await evmScriptReg.getScriptExecutor(createExecutorId(installedExecutorId)), ZERO_ADDR, 'getting disabled executor should return zero addr') }) @@ -139,7 +136,7 @@ contract('EVM Script', accounts => { const receipt = await evmScriptReg.enableScriptExecutor(installedExecutorId, { from: boss }) executorEntry = await evmScriptReg.executors(installedExecutorId) - assertEvent(receipt, 'EnableExecutor') + assertAmountOfEvents(receipt, 'EnableExecutor') assert.isTrue(executorEntry[1], "executor should now be re-enabled") assert.notEqual(await evmScriptReg.getScriptExecutor(createExecutorId(installedExecutorId)), ZERO_ADDR, 'getting disabled executor should return non-zero addr') }) @@ -201,8 +198,8 @@ contract('EVM Script', accounts => { }) beforeEach(async () => { - const receipt = await dao.newAppInstance(scriptRunnerAppId, scriptRunnerAppBase.address, EMPTY_BYTES, false, { from: boss }) - scriptRunnerApp = AppStubScriptRunner.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + const receipt = await dao.newAppInstance(SCRIPT_RUNNER_APP_ID, scriptRunnerAppBase.address, EMPTY_BYTES, false, { from: boss }) + scriptRunnerApp = AppStubScriptRunner.at(getNewProxyAddress(receipt)) await scriptRunnerApp.initialize() }) @@ -226,8 +223,8 @@ contract('EVM Script', accounts => { await evmScriptReg.addScriptExecutor(scriptExecutorMock.address, { from: boss }) // Install new script runner app - const receipt = await dao.newAppInstance(scriptRunnerAppId, scriptRunnerAppBase.address, EMPTY_BYTES, false, { from: boss }) - scriptRunnerApp = AppStubScriptRunner.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + const receipt = await dao.newAppInstance(SCRIPT_RUNNER_APP_ID, scriptRunnerAppBase.address, EMPTY_BYTES, false, { from: boss }) + scriptRunnerApp = AppStubScriptRunner.at(getNewProxyAddress(receipt)) // Explicitly don't initialize the scriptRunnerApp }) @@ -247,7 +244,7 @@ contract('EVM Script', accounts => { await acl.createPermission(boss, evmScriptReg.address, REGISTRY_ADD_EXECUTOR_ROLE, boss, { from: boss }) const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorMock.address, { from: boss }) - executorSpecId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + executorSpecId = getEventArgument(receipt, 'EnableExecutor', 'executorId') }) it("can't execute disabled spec ID", async () => { @@ -272,7 +269,7 @@ contract('EVM Script', accounts => { const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorMock.address, { from: boss }) // Sanity check it's at spec ID 1 - const executorSpecId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + const executorSpecId = getEventArgument(receipt, 'EnableExecutor', 'executorId') assert.equal(executorSpecId, 1, 'EVMScriptExecutorMock should be installed as spec ID 1') }) @@ -284,10 +281,7 @@ contract('EVM Script', accounts => { // Check logs // The executor app always uses 0x for the input and the mock script executor should return // an empty bytes array if only the spec ID is given - const scriptResult = receipt.logs.filter(l => l.event == 'ScriptResult')[0] - assert.equal(scriptResult.args.script, inputScript, 'should log the same script') - assert.equal(scriptResult.args.input, EMPTY_BYTES, 'should log the same input') - assert.equal(scriptResult.args.returnData, EMPTY_BYTES, 'should log the correct return data') + assertEvent(receipt, 'ScriptResult', { script: inputScript, input: EMPTY_BYTES, returnData: EMPTY_BYTES }) }) for (const inputBytes of [ @@ -303,10 +297,7 @@ contract('EVM Script', accounts => { // Check logs // The executor app always uses 0x for the input and the mock script executor should return // the full input script since it's more than just the spec ID - const scriptResult = receipt.logs.filter(l => l.event == 'ScriptResult')[0] - assert.equal(scriptResult.args.script, inputScript, 'should log the same script') - assert.equal(scriptResult.args.input, EMPTY_BYTES, 'should log the same input') - assert.equal(scriptResult.args.returnData, inputScript, 'should log the correct return data') + assertEvent(receipt, 'ScriptResult', { script: inputScript, input: EMPTY_BYTES, returnData: inputScript }) }) } @@ -315,8 +306,7 @@ contract('EVM Script', accounts => { const receipt = await scriptRunnerApp.runScriptWithNewBytesAllocation(inputScript) // Check logs for returned bytes - const returnedBytes = receipt.logs.filter(l => l.event == 'ReturnedBytes')[0] - assert.equal(returnedBytes.args.returnedBytes, EMPTY_BYTES, 'should log the correct return data') + assertEvent(receipt, 'ReturnedBytes', { returnedBytes: EMPTY_BYTES }) }) it('properly allocates the free memory pointer after returning bytes from executor', async () => { @@ -326,8 +316,7 @@ contract('EVM Script', accounts => { const receipt = await scriptRunnerApp.runScriptWithNewBytesAllocation(inputScript) // Check logs for returned bytes - const returnedBytes = receipt.logs.filter(l => l.event == 'ReturnedBytes')[0] - assert.equal(returnedBytes.args.returnedBytes, inputScript, 'should log the correct return data') + assertEvent(receipt, 'ReturnedBytes', { returnedBytes: inputScript }) }) }) @@ -338,7 +327,7 @@ contract('EVM Script', accounts => { const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorNoReturnMock.address, { from: boss }) // Sanity check it's at spec ID 1 - const executorSpecId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + const executorSpecId = getEventArgument(receipt, 'EnableExecutor', 'executorId') assert.equal(executorSpecId, 1, 'EVMScriptExecutorNoReturnMock should be installed as spec ID 1') }) @@ -346,7 +335,7 @@ contract('EVM Script', accounts => { const inputScript = createExecutorId(1) // Should revert with invalid return - assertRevert(scriptRunnerApp.runScript(inputScript), reverts.EVMRUN_EXECUTOR_INVALID_RETURN) + await assertRevert(scriptRunnerApp.runScript(inputScript), reverts.EVMRUN_EXECUTOR_INVALID_RETURN) }) }) @@ -357,7 +346,7 @@ contract('EVM Script', accounts => { const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorRevertMock.address, { from: boss }) // Sanity check it's at spec ID 1 - const executorSpecId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + const executorSpecId = getEventArgument(receipt, 'EnableExecutor', 'executorId') assert.equal(executorSpecId, 1, 'EVMScriptExecutorRevertMock should be installed as spec ID 1') }) @@ -366,7 +355,7 @@ contract('EVM Script', accounts => { const inputScript = createExecutorId(1) // Should revert and forward the script executor's revert message - assertRevert(scriptRunnerApp.runScript(inputScript), ERROR_MOCK_REVERT) + await assertRevert(scriptRunnerApp.runScript(inputScript), ERROR_MOCK_REVERT) }) }) @@ -381,7 +370,7 @@ contract('EVM Script', accounts => { const receipt = await evmScriptReg.addScriptExecutor(callsScriptBase.address, { from: boss }) // Sanity check it's at spec ID 1 - const callsScriptExecutorId = receipt.logs.filter(l => l.event == 'EnableExecutor')[0].args.executorId + const callsScriptExecutorId = getEventArgument(receipt, 'EnableExecutor', 'executorId') assert.equal(callsScriptExecutorId, 1, 'CallsScript should be installed as spec ID 1') executionTarget = await ExecutionTarget.new() @@ -418,11 +407,8 @@ contract('EVM Script', accounts => { // Check logs // The executor app always uses 0x for the input and the calls script always returns 0x output - const scriptResult = receipt.logs.filter(l => l.event == 'ScriptResult')[0] - assert.equal(scriptResult.args.executor, await evmScriptReg.getScriptExecutor(createExecutorId(1)), 'should log the same executor') - assert.equal(scriptResult.args.script, script, 'should log the same script') - assert.equal(scriptResult.args.input, EMPTY_BYTES, 'should log the same input') - assert.equal(scriptResult.args.returnData, EMPTY_BYTES, 'should log the empty return data') + const expectedExecutor = await evmScriptReg.getScriptExecutor(createExecutorId(1)) + assertEvent(receipt, 'ScriptResult', { executor: expectedExecutor, script, input: EMPTY_BYTES, returnData: EMPTY_BYTES }) }) it("can execute if call doesn't contain blacklist addresses", async () => { diff --git a/test/contracts/factory/evm_script_factory.js b/test/contracts/factory/evm_script_factory.js index 4c81dd5a3..d75dc612d 100644 --- a/test/contracts/factory/evm_script_factory.js +++ b/test/contracts/factory/evm_script_factory.js @@ -1,5 +1,6 @@ -const { soliditySha3 } = require('web3-utils') +const { assertEvent } = require('../../helpers/assertEvent')(web3) const { createExecutorId, encodeCallScript } = require('../../helpers/evmScript') +const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') const Kernel = artifacts.require('Kernel') const ACL = artifacts.require('ACL') @@ -15,15 +16,13 @@ const EVMScriptRegistryConstantsMock = artifacts.require('EVMScriptRegistryConst const ZERO_ADDR = '0x0000000000000000000000000000000000000000' const EMPTY_BYTES = '0x' -contract('EVM Script Factory', accounts => { +contract('EVM Script Factory', ([permissionsRoot]) => { let evmScriptRegBase, callsScriptBase let daoFact, regFact, dao, acl, evmScriptReg let APP_BASES_NAMESPACE, APP_ADDR_NAMESPACE, APP_MANAGER_ROLE, CREATE_PERMISSIONS_ROLE let EVMSCRIPT_REGISTRY_APP_ID, REGISTRY_ADD_EXECUTOR_ROLE, REGISTRY_MANAGER_ROLE - const permissionsRoot = accounts[0] - - const scriptRunnerAppId = '0x1234' + const SCRIPT_RUNNER_APP_ID = '0x1234' before(async () => { const kernelBase = await Kernel.new(true) // petrify immediately @@ -47,8 +46,8 @@ contract('EVM Script Factory', accounts => { beforeEach(async () => { const receipt = await daoFact.newDAO(permissionsRoot) - dao = Kernel.at(receipt.logs.filter(l => l.event == 'DeployDAO')[0].args.dao) - evmScriptReg = EVMScriptRegistry.at(receipt.logs.filter(l => l.event == 'DeployEVMScriptRegistry')[0].args.reg) + dao = Kernel.at(getEventArgument(receipt, 'DeployDAO', 'dao')) + evmScriptReg = EVMScriptRegistry.at(getEventArgument(receipt, 'DeployEVMScriptRegistry', 'reg')) acl = ACL.at(await dao.acl()) }) @@ -86,8 +85,8 @@ contract('EVM Script Factory', accounts => { // Set up app management permissions await acl.createPermission(permissionsRoot, dao.address, APP_MANAGER_ROLE, permissionsRoot) - const receipt = await dao.newAppInstance(scriptRunnerAppId, scriptRunnerAppBase.address, EMPTY_BYTES, false) - scriptRunnerApp = AppStubScriptRunner.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + const receipt = await dao.newAppInstance(SCRIPT_RUNNER_APP_ID, scriptRunnerAppBase.address, EMPTY_BYTES, false) + scriptRunnerApp = AppStubScriptRunner.at(getNewProxyAddress(receipt)) await scriptRunnerApp.initialize() executionTarget = await ExecutionTarget.new() }) @@ -113,13 +112,9 @@ contract('EVM Script Factory', accounts => { assert.equal(await executionTarget.counter(), 1, 'should have executed action') - // Check logs // The executor always uses 0x for the input and callscripts always have 0x returns - const scriptResult = receipt.logs.filter(l => l.event == 'ScriptResult')[0] - assert.equal(scriptResult.args.executor, await evmScriptReg.getScriptExecutor(createExecutorId(1)), 'should log the same executor') - assert.equal(scriptResult.args.script, script, 'should log the same script') - assert.equal(scriptResult.args.input, EMPTY_BYTES, 'should log the same input') - assert.equal(scriptResult.args.returnData, EMPTY_BYTES, 'should log the return data') + const expectedExecutor = await evmScriptReg.getScriptExecutor(createExecutorId(1)) + assertEvent(receipt, 'ScriptResult', { executor: expectedExecutor, script, input: EMPTY_BYTES, returnData: EMPTY_BYTES }) }) }) }) diff --git a/test/contracts/kernel/kernel_acl.js b/test/contracts/kernel/kernel_acl.js index e40fc5450..e9f2fa97d 100644 --- a/test/contracts/kernel/kernel_acl.js +++ b/test/contracts/kernel/kernel_acl.js @@ -1,9 +1,9 @@ -const assertEvent = require('../../helpers/assertEvent') -const { assertRevert } = require('../../helpers/assertThrow') const { hash } = require('eth-ens-namehash') +const { keccak_256 } = require('js-sha3') const { soliditySha3 } = require('web3-utils') -const keccak_256 = require('js-sha3').keccak_256 -const keccak256 = (name) => '0x' + keccak_256(name) +const { assertRevert } = require('../../helpers/assertThrow') +const { assertAmountOfEvents, assertEvent } = require('../../helpers/assertEvent')(web3) +const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') @@ -11,11 +11,13 @@ const KernelProxy = artifacts.require('KernelProxy') // Mocks const AppStub = artifacts.require('AppStub') -const APP_ID = hash('stub.aragonpm.test') const EMPTY_BYTES = '0x' const ZERO_ADDR = '0x0000000000000000000000000000000000000000' +const APP_ID = hash('stub.aragonpm.test') +const keccak256 = name => `0x${keccak_256(name)}` + contract('Kernel ACL', accounts => { let aclBase, appBase let APP_MANAGER_ROLE, APP_BASES_NAMESPACE, DEFAULT_ACL_APP_ID, ANY_ENTITY @@ -57,7 +59,7 @@ contract('Kernel ACL', accounts => { kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address) } - await kernel.initialize(aclBase.address, permissionsRoot); + await kernel.initialize(aclBase.address, permissionsRoot) acl = ACL.at(await kernel.acl()) kernelAddr = kernel.address }) @@ -65,20 +67,16 @@ contract('Kernel ACL', accounts => { it('cannot initialize base ACL', async () => { const newAcl = await ACL.new() assert.isTrue(await newAcl.isPetrified()) - return assertRevert(async () => { - await newAcl.initialize(permissionsRoot) - }) + await assertRevert(newAcl.initialize(permissionsRoot)) }) it('cannot initialize proxied ACL outside of Kernel', async () => { // Set up ACL proxy await acl.createPermission(permissionsRoot, kernelAddr, APP_MANAGER_ROLE, permissionsRoot) const receipt = await kernel.newAppInstance(DEFAULT_ACL_APP_ID, aclBase.address, EMPTY_BYTES, false) - const newAcl = ACL.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + const newAcl = ACL.at(getNewProxyAddress(receipt)) - return assertRevert(async () => { - await newAcl.initialize(permissionsRoot) - }) + await assertRevert(newAcl.initialize(permissionsRoot)) }) it('cannot perform actions by default', async () => { @@ -86,9 +84,7 @@ contract('Kernel ACL', accounts => { }) it('cannot perform protected actions if not allowed', async () => { - return assertRevert(async () => { - await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address, { from: noPermissions }) - }) + await assertRevert(kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address, { from: noPermissions })) }) it('create permission action can be performed by root by default', async () => { @@ -97,17 +93,15 @@ contract('Kernel ACL', accounts => { }) it('cannot create permissions without permission', async () => { - return assertRevert(async () => { - await acl.createPermission(granted, noPermissions, APP_MANAGER_ROLE, granted, { from: noPermissions }) - }) + await assertRevert(acl.createPermission(granted, noPermissions, APP_MANAGER_ROLE, granted, { from: noPermissions })) }) context('> creating permission', () => { beforeEach(async () => { const receipt = await acl.createPermission(granted, kernelAddr, APP_MANAGER_ROLE, granted, { from: permissionsRoot }) - assertEvent(receipt, 'SetPermission') - assertEvent(receipt, 'SetPermissionParams', 0) // should not have emitted this - assertEvent(receipt, 'ChangePermissionManager') + assertAmountOfEvents(receipt, 'SetPermission') + assertAmountOfEvents(receipt, 'SetPermissionParams', 0) // should not have emitted this + assertAmountOfEvents(receipt, 'ChangePermissionManager') }) it('has permission', async () => { @@ -116,7 +110,7 @@ contract('Kernel ACL', accounts => { it('can execute action', async () => { const receipt = await kernel.setApp('0x1234', APP_ID, appBase.address, { from: granted }) - assertEvent(receipt, 'SetApp') + assertAmountOfEvents(receipt, 'SetApp') }) it('can grant permission with params', async () => { @@ -141,9 +135,9 @@ contract('Kernel ACL', accounts => { assert.equal(returnedParam[2].valueOf(), parseInt(value, 10), 'param value should match') // Assert that the right events have been emitted with the right args - assertEvent(grantChildReceipt, 'SetPermission') - assertEvent(grantChildReceipt, 'SetPermissionParams') - const setParamsHash = grantChildReceipt.logs.filter(l => l.event == 'SetPermissionParams')[0].args.paramsHash + assertAmountOfEvents(grantChildReceipt, 'SetPermission') + assertAmountOfEvents(grantChildReceipt, 'SetPermissionParams') + const setParamsHash = getEventArgument(grantChildReceipt, 'SetPermissionParams', 'paramsHash') assert.equal(setParamsHash, soliditySha3(param)) // Grants again without re-saving params (saves gas) @@ -157,35 +151,31 @@ contract('Kernel ACL', accounts => { // Allows setting code for namespace other than 0 for (const grantee of [child, secondChild]) { const receipt = await kernel.setApp('0x121212', APP_ID, appBase.address, { from: grantee }) - assertEvent(receipt, 'SetApp') + assertAmountOfEvents(receipt, 'SetApp') } // Fail if setting code for namespace 0 for (const grantee of [child, secondChild]) { - await assertRevert(async () => { - await kernel.setApp('0x00', APP_ID, appBase.address, { from: grantee }) - }) + await assertRevert(kernel.setApp('0x00', APP_ID, appBase.address, { from: grantee })) } // Fail if setting code for empty namespace (which becomes 0) for (const grantee of [child, secondChild]) { - await assertRevert(async () => { - await kernel.setApp(EMPTY_BYTES, APP_ID, appBase.address, { from: grantee }) - }) + await assertRevert(kernel.setApp(EMPTY_BYTES, APP_ID, appBase.address, { from: grantee })) } }) it('can grant a public permission', async () => { const receipt = await acl.grantPermission(ANY_ENTITY, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - assertEvent(receipt, 'SetPermission') - assertEvent(receipt, 'SetPermissionParams', 0) // should not have emitted this + assertAmountOfEvents(receipt, 'SetPermission') + assertAmountOfEvents(receipt, 'SetPermissionParams', 0) // should not have emitted this // Any entity can succesfully perform action for (const granteeIndex of [4, 5, 6]) { const grantee = accounts[granteeIndex] assert.isTrue(await acl.hasPermission(grantee, kernelAddr, APP_MANAGER_ROLE), `account[${granteeIndex}] should have perm`) const setReceipt = await kernel.setApp('0x121212', APP_ID, appBase.address, { from: grantee }) - assertEvent(setReceipt, 'SetApp') + assertAmountOfEvents(setReceipt, 'SetApp') } }) @@ -198,23 +188,17 @@ contract('Kernel ACL', accounts => { }) it('root cannot revoke permission', async () => { - return assertRevert(async () => { - await acl.revokePermission(granted, kernelAddr, APP_MANAGER_ROLE, { from: permissionsRoot }) - }) + await assertRevert(acl.revokePermission(granted, kernelAddr, APP_MANAGER_ROLE, { from: permissionsRoot })) }) it('root cannot re-create permission', async () => { - return assertRevert(async () => { - await acl.createPermission(granted, kernelAddr, APP_MANAGER_ROLE, granted, { from: permissionsRoot }) - }) + await assertRevert(acl.createPermission(granted, kernelAddr, APP_MANAGER_ROLE, granted, { from: permissionsRoot })) }) it('root cannot grant permission', async () => { // Make sure child doesn't have permission yet assert.isFalse(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) - return assertRevert(async () => { - await acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: permissionsRoot }) - }) + await assertRevert(acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: permissionsRoot })) }) context('> transferring managership', () => { @@ -223,7 +207,7 @@ contract('Kernel ACL', accounts => { beforeEach(async () => { const receipt = await acl.setPermissionManager(newManager, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - assertEvent(receipt, 'ChangePermissionManager') + assertAmountOfEvents(receipt, 'ChangePermissionManager') }) it('changes manager', async () => { @@ -233,7 +217,7 @@ contract('Kernel ACL', accounts => { it('can grant permission', async () => { const receipt = await acl.grantPermission(newManager, kernelAddr, APP_MANAGER_ROLE, { from: newManager }) - assertEvent(receipt, 'SetPermission') + assertAmountOfEvents(receipt, 'SetPermission') }) it("new manager doesn't have permission yet", async () => { @@ -242,9 +226,7 @@ contract('Kernel ACL', accounts => { }) it('old manager lost power', async () => { - return assertRevert(async () => { - await acl.grantPermission(newManager, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - }) + await assertRevert(acl.grantPermission(newManager, kernelAddr, APP_MANAGER_ROLE, { from: granted })) }) }) @@ -254,7 +236,7 @@ contract('Kernel ACL', accounts => { beforeEach(async () => { const receipt = await acl.removePermissionManager(kernelAddr, APP_MANAGER_ROLE, { from: granted }) - assertEvent(receipt, 'ChangePermissionManager') + assertAmountOfEvents(receipt, 'ChangePermissionManager') }) it('removes manager', async () => { @@ -263,38 +245,32 @@ contract('Kernel ACL', accounts => { }) it('old manager lost power', async () => { - return assertRevert(async () => { - await acl.grantPermission(newManager, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - }) + await assertRevert(acl.grantPermission(newManager, kernelAddr, APP_MANAGER_ROLE, { from: granted })) }) it('can recreate permission', async () => { const createReceipt = await acl.createPermission(newManager, kernelAddr, APP_MANAGER_ROLE, newManager, { from: permissionsRoot }) - assertEvent(createReceipt, 'SetPermission') - assertEvent(createReceipt, 'ChangePermissionManager') + assertAmountOfEvents(createReceipt, 'SetPermission') + assertAmountOfEvents(createReceipt, 'ChangePermissionManager') const grantReceipt = await acl.grantPermission(granted, kernelAddr, APP_MANAGER_ROLE, { from: newManager }) - assertEvent(grantReceipt, 'SetPermission') + assertAmountOfEvents(grantReceipt, 'SetPermission') }) }) context('> self-revokes permission', () => { beforeEach(async () => { const receipt = await acl.revokePermission(granted, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - assertEvent(receipt, 'SetPermission') + assertAmountOfEvents(receipt, 'SetPermission') }) it('can no longer perform action', async () => { assert.isFalse(await acl.hasPermission(granted, kernelAddr, APP_MANAGER_ROLE)) - await assertRevert(async () => { - await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address, { from: granted }) - }) + await assertRevert(kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address, { from: granted })) }) it('permissions root cannot re-create', async () => { - return assertRevert(async () => { - await acl.createPermission(granted, kernelAddr, APP_MANAGER_ROLE, granted, { from: permissionsRoot }) - }) + await assertRevert(acl.createPermission(granted, kernelAddr, APP_MANAGER_ROLE, granted, { from: permissionsRoot })) }) it('permission manager can grant the permission', async () => { @@ -306,27 +282,25 @@ contract('Kernel ACL', accounts => { context('> re-grants to child', () => { beforeEach(async () => { const receipt = await acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - assertEvent(receipt, 'SetPermission') + assertAmountOfEvents(receipt, 'SetPermission') }) it('child entity can perform action', async () => { assert.isTrue(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) const receipt = await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address, { from: child }) - assertEvent(receipt, 'SetApp') + assertAmountOfEvents(receipt, 'SetApp') }) it('child cannot re-grant permission', async () => { const grandchild = accounts[3] // Make sure grandchild doesn't have permission yet assert.isFalse(await acl.hasPermission(grandchild, kernelAddr, APP_MANAGER_ROLE)) - return assertRevert(async () => { - await acl.grantPermission(grandchild, kernelAddr, APP_MANAGER_ROLE, { from: child }) - }) + await assertRevert(acl.grantPermission(grandchild, kernelAddr, APP_MANAGER_ROLE, { from: child })) }) it('parent can revoke permission', async () => { const receipt = await acl.revokePermission(child, kernelAddr, APP_MANAGER_ROLE, { from: granted }) - assertEvent(receipt, 'SetPermission') + assertAmountOfEvents(receipt, 'SetPermission') assert.isFalse(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) }) }) @@ -346,79 +320,53 @@ contract('Kernel ACL', accounts => { // burn it const receipt = await acl.burnPermissionManager(kernelAddr, MOCK_ROLE, { from: granted }) - const events = assertEvent(receipt, 'ChangePermissionManager') - assert.equal(events[0].args.app, kernelAddr) - assert.equal(events[0].args.role, MOCK_ROLE) - assert.equal(events[0].args.manager, BURN_ENTITY) + assertAmountOfEvents(receipt, 'ChangePermissionManager') + assertEvent(receipt, 'ChangePermissionManager', { app: kernelAddr, role: MOCK_ROLE, manager: BURN_ENTITY }) assert.equal(await acl.getPermissionManager(kernelAddr, MOCK_ROLE), BURN_ENTITY) // check that nothing else can be done from now on assert.isTrue(await acl.hasPermission(granted, kernelAddr, MOCK_ROLE)) - await assertRevert(async () => { - await acl.grantPermission(child, kernelAddr, MOCK_ROLE, { from: granted }) - }) - await assertRevert(async () => { - await acl.revokePermission(granted, kernelAddr, MOCK_ROLE, { from: granted }) - }) - await assertRevert(async () => { - await acl.setPermissionManager(granted, kernelAddr, MOCK_ROLE, { from: granted }) - }) - await assertRevert(async () => { - await acl.removePermissionManager(kernelAddr, MOCK_ROLE, { from: granted }) - }) + await assertRevert(acl.grantPermission(child, kernelAddr, MOCK_ROLE, { from: granted })) + await assertRevert(acl.revokePermission(granted, kernelAddr, MOCK_ROLE, { from: granted })) + await assertRevert(acl.setPermissionManager(granted, kernelAddr, MOCK_ROLE, { from: granted })) + await assertRevert(acl.removePermissionManager(kernelAddr, MOCK_ROLE, { from: granted })) }) it('burns non-existing permission', async () => { // burn it const receipt = await acl.createBurnedPermission(kernelAddr, MOCK_ROLE, { from: permissionsRoot }) - const events = assertEvent(receipt, 'ChangePermissionManager') - assert.equal(events[0].args.app, kernelAddr) - assert.equal(events[0].args.role, MOCK_ROLE) - assert.equal(events[0].args.manager, BURN_ENTITY) + assertAmountOfEvents(receipt, 'ChangePermissionManager') + assertEvent(receipt, 'ChangePermissionManager', { app: kernelAddr, role: MOCK_ROLE, manager: BURN_ENTITY }) assert.equal(await acl.getPermissionManager(kernelAddr, MOCK_ROLE), BURN_ENTITY) // check that nothing else can be done from now on assert.isFalse(await acl.hasPermission(granted, kernelAddr, MOCK_ROLE)) - await assertRevert(async () => { - await acl.grantPermission(child, kernelAddr, MOCK_ROLE, { from: granted }) - }) - await assertRevert(async () => { - await acl.revokePermission(granted, kernelAddr, MOCK_ROLE, { from: granted }) - }) - await assertRevert(async () => { - await acl.setPermissionManager(granted, kernelAddr, MOCK_ROLE, { from: granted }) - }) - await assertRevert(async () => { - await acl.removePermissionManager(kernelAddr, MOCK_ROLE, { from: granted }) - }) + await assertRevert(acl.grantPermission(child, kernelAddr, MOCK_ROLE, { from: granted })) + await assertRevert(acl.revokePermission(granted, kernelAddr, MOCK_ROLE, { from: granted })) + await assertRevert(acl.setPermissionManager(granted, kernelAddr, MOCK_ROLE, { from: granted })) + await assertRevert(acl.removePermissionManager(kernelAddr, MOCK_ROLE, { from: granted })) }) it('fails burning existing permission by no manager', async () => { // create permission await acl.createPermission(granted, kernelAddr, MOCK_ROLE, granted, { from: permissionsRoot }) - return assertRevert(async () => { - // try to burn it - await acl.burnPermissionManager(kernelAddr, MOCK_ROLE, { from: noPermissions }) - }) + // try to burn it + await assertRevert(acl.burnPermissionManager(kernelAddr, MOCK_ROLE, { from: noPermissions })) }) it('fails trying to create a burned permission which already has a manager', async () => { // create permission await acl.createPermission(granted, kernelAddr, MOCK_ROLE, granted, { from: permissionsRoot }) - await assertRevert(async () => { - // try to create it burnt - await acl.createBurnedPermission(kernelAddr, MOCK_ROLE, { from: permissionsRoot }) - }) + // try to create it burnt + await assertRevert(acl.createBurnedPermission(kernelAddr, MOCK_ROLE, { from: permissionsRoot })) // even removing the only grantee, still fails await acl.revokePermission(granted, kernelAddr, MOCK_ROLE, { from: granted }) - return assertRevert(async () => { - // try to create it burnt - await acl.createBurnedPermission(kernelAddr, MOCK_ROLE, { from: permissionsRoot }) - }) + // try to create it burnt + await assertRevert(acl.createBurnedPermission(kernelAddr, MOCK_ROLE, { from: permissionsRoot })) }) }) }) diff --git a/test/contracts/kernel/kernel_apps.js b/test/contracts/kernel/kernel_apps.js index 6e6610035..49fea658b 100644 --- a/test/contracts/kernel/kernel_apps.js +++ b/test/contracts/kernel/kernel_apps.js @@ -1,7 +1,8 @@ -const { assertRevert } = require('../../helpers/assertThrow') -const { onlyIf } = require('../../helpers/onlyIf') -const { getBalance } = require('../../helpers/web3') const { hash } = require('eth-ens-namehash') +const { onlyIf } = require('../../helpers/onlyIf') +const { assertRevert } = require('../../helpers/assertThrow') +const { getNewProxyAddress } = require('../../helpers/events') +const { assertAmountOfEvents } = require('../../helpers/assertEvent')(web3) const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') @@ -19,13 +20,11 @@ const APP_ID = hash('stub.aragonpm.test') const EMPTY_BYTES = '0x' const ZERO_ADDR = '0x0000000000000000000000000000000000000000' -contract('Kernel apps', accounts => { +contract('Kernel apps', ([permissionsRoot]) => { let aclBase, appBase1, appBase2 - let APP_BASES_NAMESPACE, APP_ADDR_NAMESPACE + let APP_BASES_NAMESPACE, APP_ADDR_NAMESPACE, APP_MANAGER_ROLE let UPGRADEABLE, FORWARDING - const permissionsRoot = accounts[0] - // Initial setup before(async () => { aclBase = await ACL.new() @@ -62,7 +61,7 @@ contract('Kernel apps', accounts => { kernel = Kernel.at((await KernelProxy.new(kernelBase.address)).address) } - await kernel.initialize(aclBase.address, permissionsRoot); + await kernel.initialize(aclBase.address, permissionsRoot) acl = ACL.at(await kernel.acl()) await acl.createPermission(permissionsRoot, kernel.address, APP_MANAGER_ROLE, permissionsRoot) }) @@ -71,15 +70,11 @@ contract('Kernel apps', accounts => { * TESTS * *********/ it('fails if setting app to 0 address', async () => { - return assertRevert(async () => { - await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, ZERO_ADDR) - }) + await assertRevert(kernel.setApp(APP_BASES_NAMESPACE, APP_ID, ZERO_ADDR)) }) it('fails if setting app to non-contract address', async () => { - return assertRevert(async () => { - await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, '0x1234') - }) + await assertRevert(kernel.setApp(APP_BASES_NAMESPACE, APP_ID, '0x1234')) }) const newAppProxyMapping = { @@ -102,7 +97,7 @@ contract('Kernel apps', accounts => { onlyAppProxy(() => it('creates a new upgradeable app proxy instance', async () => { const receipt = await kernel.newAppInstance(APP_ID, appBase1.address, EMPTY_BYTES, false) - const appProxy = AppProxyUpgradeable.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + const appProxy = AppProxyUpgradeable.at(getNewProxyAddress(receipt)) assert.equal(await appProxy.kernel(), kernel.address, "new appProxy instance's kernel should be set to the originating kernel") // Checks ERC897 functionality @@ -114,7 +109,7 @@ contract('Kernel apps', accounts => { onlyAppProxyPinned(() => it('creates a new non upgradeable app proxy instance', async () => { const receipt = await kernel.newPinnedAppInstance(APP_ID, appBase1.address) - const appProxy = AppProxyPinned.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy) + const appProxy = AppProxyPinned.at(getNewProxyAddress(receipt)) assert.equal(await appProxy.kernel(), kernel.address, "new appProxy instance's kernel should be set to the originating kernel") // Checks ERC897 functionality @@ -144,12 +139,12 @@ contract('Kernel apps', accounts => { it("doesn't set the app base when already set", async() => { await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase1.address) const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address, EMPTY_BYTES, false) - assert.isFalse(receipt.logs.includes(l => l.event == 'SetApp')) + assertAmountOfEvents(receipt, 'SetApp', 0) }) it("also sets the default app", async () => { const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address, EMPTY_BYTES, true) - const appProxyAddr = receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy + const appProxyAddr = getNewProxyAddress(receipt) // Check that both the app base and default app are set assert.equal(await kernel.getApp(APP_BASES_NAMESPACE, APP_ID), appBase1.address, 'App base should be set') @@ -162,11 +157,10 @@ contract('Kernel apps', accounts => { it("allows initializing proxy", async () => { const initData = appBase1.initialize.request().params[0].data - const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address, initData, false) - const appProxyAddr = receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy - // Make sure app was initialized - // assert.isTrue(await AppStub.at(appProxyAddr).hasInitialized(), 'App should have been initialized') + const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address, initData, false) + const appProxyAddr = getNewProxyAddress(receipt) + assert.isTrue(await AppStub.at(appProxyAddr).hasInitialized(), 'App should have been initialized') // Check that the app base has been set, but the app isn't the default app assert.equal(await kernel.getApp(APP_BASES_NAMESPACE, APP_ID), appBase1.address, 'App base should be set') @@ -174,9 +168,7 @@ contract('Kernel apps', accounts => { }) it("fails if the app base is not given", async() => { - return assertRevert(async () => { - await kernelOverload[newInstanceFn](APP_ID, ZERO_ADDR, EMPTY_BYTES, false) - }) + await assertRevert(kernelOverload[newInstanceFn](APP_ID, ZERO_ADDR, EMPTY_BYTES, false)) }) it('fails if the given app base is different than the existing one', async() => { @@ -185,9 +177,7 @@ contract('Kernel apps', accounts => { assert.notEqual(existingBase, differentBase, 'appBase1 and appBase2 should have different addresses') await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, existingBase) - return assertRevert(async () => { - await kernelOverload[newInstanceFn](APP_ID, differentBase, EMPTY_BYTES, false) - }) + await assertRevert(kernelOverload[newInstanceFn](APP_ID, differentBase, EMPTY_BYTES, false)) }) }) @@ -212,12 +202,12 @@ contract('Kernel apps', accounts => { it("doesn't set the app base when already set", async() => { await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase1.address) const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address) - assert.isFalse(receipt.logs.includes(l => l.event == 'SetApp')) + assertAmountOfEvents(receipt, 'SetApp', 0) }) it("does not set the default app", async () => { const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address) - const appProxyAddr = receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy + const appProxyAddr = getNewProxyAddress(receipt) // Check that only the app base is set assert.equal(await kernel.getApp(APP_BASES_NAMESPACE, APP_ID), appBase1.address, 'App base should be set') @@ -228,10 +218,8 @@ contract('Kernel apps', accounts => { }) it("does not allow initializing proxy", async () => { - const initData = appBase1.initialize.request().params[0].data - const receipt = await kernelOverload[newInstanceFn](APP_ID, appBase1.address) - const appProxyAddr = receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy + const appProxyAddr = getNewProxyAddress(receipt) // Make sure app was not initialized assert.isFalse(await AppStub.at(appProxyAddr).hasInitialized(), 'App should have been initialized') @@ -242,9 +230,7 @@ contract('Kernel apps', accounts => { }) it("fails if the app base is not given", async() => { - return assertRevert(async () => { - await kernelOverload[newInstanceFn](APP_ID, ZERO_ADDR) - }) + await assertRevert(kernelOverload[newInstanceFn](APP_ID, ZERO_ADDR)) }) it('fails if the given app base is different than the existing one', async() => { @@ -253,9 +239,7 @@ contract('Kernel apps', accounts => { assert.notEqual(existingBase, differentBase, 'appBase1 and appBase2 should have different addresses') await kernel.setApp(APP_BASES_NAMESPACE, APP_ID, existingBase) - return assertRevert(async () => { - await kernelOverload[newInstanceFn](APP_ID, differentBase) - }) + await assertRevert(kernelOverload[newInstanceFn](APP_ID, differentBase)) }) }) }) diff --git a/test/contracts/kernel/kernel_funds.js b/test/contracts/kernel/kernel_funds.js index 4f47636eb..8a6c044a1 100644 --- a/test/contracts/kernel/kernel_funds.js +++ b/test/contracts/kernel/kernel_funds.js @@ -1,6 +1,6 @@ -const { assertRevert } = require('../../helpers/assertThrow') -const { getBalance } = require('../../helpers/web3') 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') @@ -11,9 +11,8 @@ const KernelDepositableMock = artifacts.require('KernelDepositableMock') const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies -contract('Kernel funds', accounts => { +contract('Kernel funds', ([permissionsRoot]) => { let aclBase - const permissionsRoot = accounts[0] // Initial setup before(async () => { @@ -48,17 +47,13 @@ contract('Kernel funds', accounts => { // Before initialization assert.isFalse(await kernel.hasInitialized(), 'should not have been initialized') - await assertRevert(async () => { - await kernel.sendTransaction({ value: 1, gas: SEND_ETH_GAS }) - }) + await assertRevert(kernel.sendTransaction({ value: 1, gas: SEND_ETH_GAS })) // After initialization - await kernel.initialize(aclBase.address, permissionsRoot); + await kernel.initialize(aclBase.address, permissionsRoot) assert.isTrue(await kernel.hasInitialized(), 'should have been initialized') - await assertRevert(async () => { - await kernel.sendTransaction({ value: 1, gas: SEND_ETH_GAS }) - }) + await assertRevert(kernel.sendTransaction({ value: 1, gas: SEND_ETH_GAS })) }) onlyKernelDepositable(() => { @@ -68,7 +63,7 @@ contract('Kernel funds', accounts => { assert.isFalse(await kernel.isDepositable(), 'should not be depositable') // After initialization - await kernel.initialize(aclBase.address, permissionsRoot); + await kernel.initialize(aclBase.address, permissionsRoot) assert.isTrue(await kernel.hasInitialized(), 'should have been initialized') assert.isFalse(await kernel.isDepositable(), 'should not be depositable') }) @@ -77,8 +72,8 @@ contract('Kernel funds', accounts => { const amount = 1 const initialBalance = await getBalance(kernel.address) - await kernel.initialize(aclBase.address, permissionsRoot); - await kernel.enableDeposits(); + await kernel.initialize(aclBase.address, permissionsRoot) + await kernel.enableDeposits() assert.isTrue(await kernel.hasInitialized(), 'should have been initialized') assert.isTrue(await kernel.isDepositable(), 'should be depositable') diff --git a/test/contracts/kernel/kernel_lifecycle.js b/test/contracts/kernel/kernel_lifecycle.js index 60c46007d..832a05295 100644 --- a/test/contracts/kernel/kernel_lifecycle.js +++ b/test/contracts/kernel/kernel_lifecycle.js @@ -1,7 +1,7 @@ -const assertEvent = require('../../helpers/assertEvent') +const { hash } = require('eth-ens-namehash') const { assertRevert } = require('../../helpers/assertThrow') const { getBlockNumber } = require('../../helpers/web3') -const { hash } = require('eth-ens-namehash') +const { assertEvent, assertAmountOfEvents } = require('../../helpers/assertEvent')(web3) const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') @@ -13,31 +13,31 @@ const APP_ID = hash('stub.aragonpm.test') const VAULT_ID = hash('vault.aragonpm.test') const EMPTY_BYTES = '0x' -contract('Kernel lifecycle', accounts => { +contract('Kernel lifecycle', ([root, someone]) => { let aclBase, appBase let DEFAULT_ACL_APP_ID, APP_BASES_NAMESPACE, APP_ADDR_NAMESPACE, APP_MANAGER_ROLE const testUnaccessibleFunctionalityWhenUninitialized = async (kernel) => { // hasPermission should always return false when uninitialized - assert.isFalse(await kernel.hasPermission(accounts[0], kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) - assert.isFalse(await kernel.hasPermission(accounts[1], kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) + assert.isFalse(await kernel.hasPermission(root, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) + assert.isFalse(await kernel.hasPermission(someone, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) - await assertRevert(() => kernel.newAppInstance(APP_ID, appBase.address, EMPTY_BYTES, false)) - await assertRevert(() => kernel.newPinnedAppInstance(APP_ID, appBase.address)) - await assertRevert(() => kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address)) - await assertRevert(() => kernel.setRecoveryVaultAppId(VAULT_ID)) + await assertRevert(kernel.newAppInstance(APP_ID, appBase.address, EMPTY_BYTES, false)) + await assertRevert(kernel.newPinnedAppInstance(APP_ID, appBase.address)) + await assertRevert(kernel.setApp(APP_BASES_NAMESPACE, APP_ID, appBase.address)) + await assertRevert(kernel.setRecoveryVaultAppId(VAULT_ID)) } const testUsability = async (kernel) => { // Make sure we haven't already setup the required permission - assert.isFalse(await kernel.hasPermission(accounts[0], kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) - assert.isFalse(await kernel.hasPermission(accounts[1], kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) + assert.isFalse(await kernel.hasPermission(root, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) + assert.isFalse(await kernel.hasPermission(someone, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) // Then set the required permission const acl = ACL.at(await kernel.acl()) - await acl.createPermission(accounts[0], kernel.address, APP_MANAGER_ROLE, accounts[0]) - assert.isTrue(await kernel.hasPermission(accounts[0], kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) - assert.isFalse(await kernel.hasPermission(accounts[1], kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) + await acl.createPermission(root, kernel.address, APP_MANAGER_ROLE, root) + assert.isTrue(await kernel.hasPermission(root, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) + assert.isFalse(await kernel.hasPermission(someone, kernel.address, APP_MANAGER_ROLE, EMPTY_BYTES)) // And finally test functionality await kernel.newAppInstance(APP_ID, appBase.address, EMPTY_BYTES, false) @@ -73,9 +73,7 @@ contract('Kernel lifecycle', accounts => { }) it('throws on initialization', async () => { - await assertRevert(async () => { - await kernelBase.initialize(accounts[0], accounts[0]) - }) + await assertRevert(kernelBase.initialize(root, root)) }) it('should not be usable', async () => { @@ -99,7 +97,7 @@ contract('Kernel lifecycle', accounts => { }) it('is initializable and usable', async () => { - await kernel.initialize(aclBase.address, accounts[0]) + await kernel.initialize(aclBase.address, root) assert.isTrue(await kernel.hasInitialized(), 'should be initialized') assert.isFalse(await kernel.isPetrified(), 'should not be petrified') @@ -133,25 +131,17 @@ contract('Kernel lifecycle', accounts => { }) context('> Initialized', () => { - let initReceipt + let initReceipt, acl beforeEach(async () => { - initReceipt = await kernel.initialize(aclBase.address, accounts[0]) + initReceipt = await kernel.initialize(aclBase.address, root) acl = ACL.at(await kernel.acl()) }) it('set the ACL correctly', async () => { - assertEvent(initReceipt, 'SetApp', 2) - const setAppLogs = initReceipt.logs.filter(l => l.event == 'SetApp') - const ACLBaseLog = setAppLogs[0] - const ACLAppLog = setAppLogs[1] - - assert.equal(ACLBaseLog.args.namespace, APP_BASES_NAMESPACE, 'should set base acl first') - assert.equal(ACLBaseLog.args.appId, DEFAULT_ACL_APP_ID, 'should set base acl first') - assert.equal(ACLBaseLog.args.app, aclBase.address, 'should set base acl first') - assert.equal(ACLAppLog.args.namespace, APP_ADDR_NAMESPACE, 'should set default acl second') - assert.equal(ACLAppLog.args.appId, DEFAULT_ACL_APP_ID, 'should set default acl second') - assert.equal(ACLAppLog.args.app, acl.address, 'should set default acl second') + assertAmountOfEvents(initReceipt, 'SetApp', 2) + assertEvent(initReceipt, 'SetApp', { namespace: APP_BASES_NAMESPACE, appId: DEFAULT_ACL_APP_ID, app: aclBase.address }, 0) + assertEvent(initReceipt, 'SetApp', { namespace: APP_ADDR_NAMESPACE, appId: DEFAULT_ACL_APP_ID, app: acl.address }, 1) }) it('is now initialized', async () => { @@ -167,9 +157,7 @@ contract('Kernel lifecycle', accounts => { }) it('throws on reinitialization', async () => { - return assertRevert(async () => { - await kernel.initialize(accounts[0], accounts[0]) - }) + await assertRevert(kernel.initialize(root, root)) }) it('should now be usable', async () => { diff --git a/test/contracts/kernel/kernel_upgrade.js b/test/contracts/kernel/kernel_upgrade.js index b7a997aa1..04df67a20 100644 --- a/test/contracts/kernel/kernel_upgrade.js +++ b/test/contracts/kernel/kernel_upgrade.js @@ -62,27 +62,19 @@ contract('Kernel upgrade', accounts => { }) it('fails to create a KernelProxy if the base is 0', async () => { - return assertRevert(async () => { - await KernelProxy.new(ZERO_ADDR) - }) + await assertRevert(KernelProxy.new(ZERO_ADDR)) }) it('fails to create a KernelProxy if the base is not a contract', async () => { - return assertRevert(async () => { - await KernelProxy.new('0x1234') - }) + await assertRevert(KernelProxy.new('0x1234')) }) it('fails to upgrade kernel without permission', async () => { - return assertRevert(async () => { - await kernel.setApp(CORE_NAMESPACE, KERNEL_APP_ID, accounts[0]) - }) + await assertRevert(kernel.setApp(CORE_NAMESPACE, KERNEL_APP_ID, accounts[0])) }) it('fails when calling upgraded functionality on old version', async () => { - return assertRevert(async () => { - await UpgradedKernel.at(kernelAddr).isUpgraded() - }) + await assertRevert(UpgradedKernel.at(kernelAddr).isUpgraded()) }) it('successfully upgrades kernel', async () => { @@ -96,8 +88,6 @@ contract('Kernel upgrade', accounts => { it('fails if upgrading to kernel that is not a contract', async () => { await acl.createPermission(permissionsRoot, kernelAddr, APP_MANAGER_ROLE, permissionsRoot, { from: permissionsRoot }) - return assertRevert(async () => { - await kernel.setApp(CORE_NAMESPACE, KERNEL_APP_ID, '0x1234') - }) + await assertRevert(kernel.setApp(CORE_NAMESPACE, KERNEL_APP_ID, '0x1234')) }) }) diff --git a/test/contracts/lib/math/lib_safemath64.js b/test/contracts/lib/math/lib_safemath64.js index 07a1b9b25..d6619ff2e 100644 --- a/test/contracts/lib/math/lib_safemath64.js +++ b/test/contracts/lib/math/lib_safemath64.js @@ -1,6 +1,6 @@ const { assertRevert } = require('../../../helpers/assertThrow') -contract('SafeMath64 lib test', accounts => { +contract('SafeMath64 lib test', () => { let safeMath64Mock before(async () => { @@ -11,74 +11,64 @@ contract('SafeMath64 lib test', accounts => { it('multiplies', async () => { const a = 1234 const b = 32746 - assert.equal((await safeMath64Mock.mulExt(a, b)).toString(), a * b, "Values should match") + assert.equal((await safeMath64Mock.mulExt(a, b)).toString(), a * b, 'values should match') }) it('fails if multplication overflows', async () => { const a = new web3.BigNumber(2).pow(63) const b = 2 - return assertRevert(async () => { - await safeMath64Mock.mulExt(a, b) - }) + await assertRevert(safeMath64Mock.mulExt(a, b)) }) // division it('divides', async () => { const a = 32746 const b = 1234 - assert.equal((await safeMath64Mock.divExt(a, b)).toString(), Math.floor(a / b), "Values should match") + assert.equal((await safeMath64Mock.divExt(a, b)).toString(), Math.floor(a / b), 'values should match') }) it('fails dividing by zero', async () => { const a = 1234 const b = 0 - return assertRevert(async () => { - await safeMath64Mock.divExt(a, b) - }) + await assertRevert(safeMath64Mock.divExt(a, b)) }) // subtraction it('subtract', async () => { const a = 1234 const b = 327 - assert.equal((await safeMath64Mock.subExt(a, b)).toString(), a - b, "Values should match") + assert.equal((await safeMath64Mock.subExt(a, b)).toString(), a - b, 'values should match') }) it('fails if subtraction underflows', async () => { const a = 123 const b = 124 - return assertRevert(async () => { - await safeMath64Mock.subExt(a, b) - }) + await assertRevert(safeMath64Mock.subExt(a, b)) }) // addition it('adds', async () => { const a = 1234 const b = 327 - assert.equal((await safeMath64Mock.addExt(a, b)).toString(), a + b, "Values should match") + assert.equal((await safeMath64Mock.addExt(a, b)).toString(), a + b, 'values should match') }) it('fails if addition overflows', async () => { const a = new web3.BigNumber(2).pow(63) const b = new web3.BigNumber(2).pow(63) - return assertRevert(async () => { - await safeMath64Mock.addExt(a, b) - }) + await assertRevert(safeMath64Mock.addExt(a, b)) }) // modulo it('divides modulo', async () => { const a = 32746 const b = 1234 - assert.equal((await safeMath64Mock.modExt(a, b)).toString(), a % b, "Values should match") + assert.equal((await safeMath64Mock.modExt(a, b)).toString(), a % b, 'values should match') }) it('fails modulo dividing by zero', async () => { const a = 1234 const b = 0 - return assertRevert(async () => { - await safeMath64Mock.modExt(a, b) - }) + await assertRevert(safeMath64Mock.modExt(a, b)) }) }) diff --git a/test/contracts/lib/math/lib_safemath8.js b/test/contracts/lib/math/lib_safemath8.js index e350fa8a4..e56869516 100644 --- a/test/contracts/lib/math/lib_safemath8.js +++ b/test/contracts/lib/math/lib_safemath8.js @@ -1,6 +1,6 @@ const { assertRevert } = require('../../../helpers/assertThrow') -contract('SafeMath8 lib test', accounts => { +contract('SafeMath8 lib test', () => { let safeMath8Mock before(async () => { @@ -11,74 +11,64 @@ contract('SafeMath8 lib test', accounts => { it('multiplies', async () => { const a = 16 const b = 15 - assert.equal((await safeMath8Mock.mulExt(a, b)).toString(), a * b, "Values should match") + assert.equal((await safeMath8Mock.mulExt(a, b)).toString(), a * b, 'values should match') }) it('fails if multplication overflows', async () => { const a = 16 const b = 16 - return assertRevert(async () => { - await safeMath8Mock.mulExt(a, b) - }) + await assertRevert(safeMath8Mock.mulExt(a, b)) }) // division it('divides', async () => { const a = 149 const b = 50 - assert.equal((await safeMath8Mock.divExt(a, b)).toString(), Math.floor(a / b), "Values should match") + assert.equal((await safeMath8Mock.divExt(a, b)).toString(), Math.floor(a / b), 'values should match') }) it('fails dividing by zero', async () => { const a = 123 const b = 0 - return assertRevert(async () => { - await safeMath8Mock.divExt(a, b) - }) + await assertRevert(safeMath8Mock.divExt(a, b)) }) // subtraction it('subtract', async () => { const a = 123 const b = 122 - assert.equal((await safeMath8Mock.subExt(a, b)).toString(), a - b, "Values should match") + assert.equal((await safeMath8Mock.subExt(a, b)).toString(), a - b, 'values should match') }) it('fails if subtraction underflows', async () => { const a = 123 const b = 124 - return assertRevert(async () => { - await safeMath8Mock.subExt(a, b) - }) + await assertRevert(safeMath8Mock.subExt(a, b)) }) // addition it('adds', async () => { const a = 128 const b = 127 - assert.equal((await safeMath8Mock.addExt(a, b)).toString(), a + b, "Values should match") + assert.equal((await safeMath8Mock.addExt(a, b)).toString(), a + b, 'values should match') }) it('fails if addition overflows', async () => { const a = 128 const b = 128 - return assertRevert(async () => { - await safeMath8Mock.addExt(a, b) - }) + await assertRevert(safeMath8Mock.addExt(a, b)) }) // modulo it('divides modulo', async () => { const a = 149 const b = 50 - assert.equal((await safeMath8Mock.modExt(a, b)).toString(), a % b, "Values should match") + assert.equal((await safeMath8Mock.modExt(a, b)).toString(), a % b, 'values should match') }) it('fails modulo dividing by zero', async () => { const a = 123 const b = 0 - return assertRevert(async () => { - await safeMath8Mock.modExt(a, b) - }) + await assertRevert(safeMath8Mock.modExt(a, b)) }) }) diff --git a/test/helpers/assertEvent.js b/test/helpers/assertEvent.js index abdc148b0..e8cabaf39 100644 --- a/test/helpers/assertEvent.js +++ b/test/helpers/assertEvent.js @@ -1,5 +1,28 @@ -module.exports = (receipt, eventName, instances = 1) => { - const events = receipt.logs.filter(x => x.event == eventName) - assert.equal(events.length, instances, `'${eventName}' event should have been fired ${instances} times`) - return events +const { getEventAt, getEvents } = require('./events') + +module.exports = web3 => { + const assertEvent = (receipt, eventName, expectedArgs = {}, index = 0) => { + const event = getEventAt(receipt, eventName, index) + assert(typeof event === 'object', `could not find an emitted ${eventName} event ${index === 0 ? '' : `at index ${index}`}`) + + for (const arg of Object.keys(expectedArgs)) { + let foundArg = event.args[arg] + if (foundArg instanceof web3.BigNumber) foundArg = foundArg.toString() + + let expectedArg = expectedArgs[arg] + if (expectedArg instanceof web3.BigNumber) expectedArg = expectedArg.toString() + + assert.equal(foundArg, expectedArg, `${eventName} event ${arg} value does not match`) + } + } + + const assertAmountOfEvents = (receipt, eventName, expectedAmount = 1) => { + const events = getEvents(receipt, eventName) + assert.equal(events.length, expectedAmount, `number of ${eventName} events does not match`) + } + + return { + assertEvent, + assertAmountOfEvents + } } diff --git a/test/helpers/events.js b/test/helpers/events.js new file mode 100644 index 000000000..aa13b1a90 --- /dev/null +++ b/test/helpers/events.js @@ -0,0 +1,11 @@ +const getEvents = ({ logs = [] }, event) => logs.filter(l => l.event === event) +const getEventAt = (receipt, event, index = 0) => getEvents(receipt, event)[index] +const getEventArgument = (receipt, event, arg, index = 0) => getEventAt(receipt, event, index).args[arg] +const getNewProxyAddress = (receipt) => getEventArgument(receipt, 'NewAppProxy', 'proxy') + +module.exports = { + getEvents, + getEventAt, + getEventArgument, + getNewProxyAddress +} From 34554a85194fbfa71b71d57ef19b8ad1610b09fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Iv=C3=A1n=20Cuende?= Date: Wed, 3 Jul 2019 11:56:29 +0200 Subject: [PATCH 07/10] Add probot configuration (#530) --- .github/auto_assign.yml | 16 ++++++++++++++++ .github/config.yml | 12 ++++++++++++ .github/release-drafter.yml | 6 ++++++ 3 files changed, 34 insertions(+) create mode 100644 .github/auto_assign.yml create mode 100644 .github/config.yml create mode 100644 .github/release-drafter.yml diff --git a/.github/auto_assign.yml b/.github/auto_assign.yml new file mode 100644 index 000000000..27cb4fe2d --- /dev/null +++ b/.github/auto_assign.yml @@ -0,0 +1,16 @@ +# Configuration for https://probot.github.io/apps/auto-assign + +addReviewers: true +addAssignees: false + +reviewers: + - sohkai + - facuspagnuolo + - izqui + +skipKeywords: + - wip + - draft + +# Set 0 to add all the reviewers (default: 0) +numberOfReviewers: 0 diff --git a/.github/config.yml b/.github/config.yml new file mode 100644 index 000000000..7a6c85711 --- /dev/null +++ b/.github/config.yml @@ -0,0 +1,12 @@ +# Configuration for https://probot.github.io/apps/welcome + +newIssueWelcomeComment: > + Thanks for opening your first issue in aragonOS! Someone will circle back soon ⚡ + +newPRWelcomeComment: > + Thanks for opening this pull request! Someone will review it soon 🔍 + +firstPRMergeComment: > + Congrats on merging your first pull request! Aragon is proud of you 🦅 + + ![Eagle gif](https://media.giphy.com/media/SLD8eKFPuUDuw/200w_d.gif) diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml new file mode 100644 index 000000000..45bee1fda --- /dev/null +++ b/.github/release-drafter.yml @@ -0,0 +1,6 @@ +# Configuration for https://probot.github.io/apps/release-drafter + +template: | + ## What’s changed in aragonOS + + $CHANGES From cfb113d983be661e65076e97858c44b3b185c378 Mon Sep 17 00:00:00 2001 From: Haythem Sellami Date: Wed, 3 Jul 2019 11:19:41 +0100 Subject: [PATCH 08/10] Add development network to truffle config (#529) --- truffle-config.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/truffle-config.js b/truffle-config.js index 0559e2106..6831f9a6c 100644 --- a/truffle-config.js +++ b/truffle-config.js @@ -98,6 +98,13 @@ module.exports = { gas: 0xffffffffff, gasPrice: 0x01 }, + development: { + host: 'localhost', + network_id: '*', + port: 8545, + gas: 6.9e6, + gasPrice: 15000000001 + }, }, build: {}, mocha, From e45412fbf5f7be257e9b9e3ab215ce8dfa6c9664 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Thu, 4 Jul 2019 21:26:38 +0200 Subject: [PATCH 09/10] Add IForwarderFee (#536) --- contracts/common/IForwarderFee.sol | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 contracts/common/IForwarderFee.sol diff --git a/contracts/common/IForwarderFee.sol b/contracts/common/IForwarderFee.sol new file mode 100644 index 000000000..80913fe64 --- /dev/null +++ b/contracts/common/IForwarderFee.sol @@ -0,0 +1,10 @@ +/* + * SPDX-License-Identitifer: MIT + */ + +pragma solidity ^0.4.24; + + +interface IForwarderFee { + function forwardFee() external view returns (address, uint256); +} From 863ceb97357c58b4a1c39a4be4d814f8a50200e4 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Thu, 11 Jul 2019 11:11:15 +0200 Subject: [PATCH 10/10] Bump to v4.2.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f6a3218ed..ca57aac09 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@aragon/os", - "version": "4.2.0", + "version": "4.2.1", "description": "Core contracts for Aragon", "scripts": { "compile": "truffle compile",