Skip to content

Commit

Permalink
Merge 8bb05ad into c4e0fb1
Browse files Browse the repository at this point in the history
  • Loading branch information
@sohkai
sohkai committed Feb 8, 2019
2 parents c4e0fb1 + 8bb05ad commit f8f5b67
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 66 deletions.
74 changes: 41 additions & 33 deletions contracts/test/mocks/APMRegistryFactoryMock.sol
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,49 @@
pragma solidity 0.4.24;

import "../../apm/APMRegistry.sol";
import "../../apm/Repo.sol";
import "../../ens/ENSSubdomainRegistrar.sol";

import "../../factory/DAOFactory.sol";
import "../../factory/ENSFactory.sol";

// Mock that doesn't grant enough permissions
// external ENS
// Only usable with new ENS instance

import "../../factory/APMRegistryFactory.sol";
contract APMRegistryFactoryMock is APMInternalAppNames {
DAOFactory public daoFactory;
APMRegistry public registryBase;
Repo public repoBase;
ENSSubdomainRegistrar public ensSubdomainRegistrarBase;
ENS public ens;

contract APMRegistryFactoryMock is APMRegistryFactory {
constructor(
DAOFactory _daoFactory,
APMRegistry _registryBase,
Repo _repoBase,
ENSSubdomainRegistrar _ensSubBase,
ENS _ens,
ENSFactory _ensFactory
)
APMRegistryFactory(_daoFactory, _registryBase, _repoBase, _ensSubBase, _ens, _ensFactory) public {}

function newAPM(bytes32, bytes32, address) public returns (APMRegistry) {}

function newBadAPM(bytes32 tld, bytes32 label, address _root, bool withoutACL) public returns (APMRegistry) {
bytes32 node = keccak256(abi.encodePacked(tld, label));
) public
{
daoFactory = _daoFactory;
registryBase = _registryBase;
repoBase = _repoBase;
ensSubdomainRegistrarBase = _ensSubBase;
ens = _ensFactory.newENS(this);
}

// Assume it is the test ENS
if (ens.owner(node) != address(this)) {
// If we weren't in test ens and factory doesn't have ownership, will fail
ens.setSubnodeOwner(tld, label, this);
}
function newFailingAPM(
bytes32 _tld,
bytes32 _label,
address _root,
bool _withoutNameRole
)
public
returns (APMRegistry)
{
// Set up ENS control
bytes32 node = keccak256(abi.encodePacked(_tld, _label));
ens.setSubnodeOwner(_tld, _label, this);

Kernel dao = daoFactory.newDAO(this);
ACL acl = ACL(dao.acl());
Expand Down Expand Up @@ -55,34 +73,24 @@ contract APMRegistryFactoryMock is APMRegistryFactory {
bytes32 repoAppId = keccak256(abi.encodePacked(node, keccak256(abi.encodePacked(REPO_APP_NAME))));
dao.setApp(dao.APP_BASES_NAMESPACE(), repoAppId, repoBase);

emit DeployAPM(node, apm);

// Grant permissions needed for APM on ENSSubdomainRegistrar
acl.createPermission(apm, ensSub, ensSub.POINT_ROOTNODE_ROLE(), _root);

if (withoutACL) {
// Don't grant all permissions needed for APM to initialize
if (_withoutNameRole) {
acl.createPermission(apm, ensSub, ensSub.CREATE_NAME_ROLE(), _root);
}

acl.createPermission(apm, ensSub, ensSub.POINT_ROOTNODE_ROLE(), _root);

configureAPMPermissions(acl, apm, _root);

// allow apm to create permissions for Repos in Kernel
bytes32 permRole = acl.CREATE_PERMISSIONS_ROLE();

if (!withoutACL) {
if (!_withoutNameRole) {
bytes32 permRole = acl.CREATE_PERMISSIONS_ROLE();
acl.grantPermission(apm, acl, permRole);
}

// Permission transition to _root
acl.setPermissionManager(_root, dao, dao.APP_MANAGER_ROLE());
acl.revokePermission(this, acl, permRole);
acl.grantPermission(_root, acl, permRole);
acl.setPermissionManager(_root, acl, permRole);

// Initialize
ens.setOwner(node, ensSub);
ensSub.initialize(ens, node);

// This should fail since we haven't given all required permissions
apm.initialize(ensSub);

return apm;
Expand Down
29 changes: 20 additions & 9 deletions test/apm_registry.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,17 @@ contract('APMRegistry', accounts => {
assert.equal(await repo.getVersionsCount(), 2, 'should have created version')
})

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 newFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ens2.address, '0x00')

// Factory doesn't have ownership over 'eth' tld
await assertRevert(async () => {
await newFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner)
})
})

it('fails to create empty repo name', async () => {
return assertRevert(async () => {
await registry.newRepo('', repoDev, { from: apmOwner })
Expand All @@ -103,7 +114,7 @@ contract('APMRegistry', accounts => {
})
})

context('creating test.aragonpm.eth repo', () => {
context('> Creating test.aragonpm.eth repo', () => {
let repo = {}

beforeEach(async () => {
Expand Down Expand Up @@ -164,22 +175,22 @@ contract('APMRegistry', accounts => {
})
})

context('APMRegistry created with lacking permissions', () => {
context('> Created with missing permissions', () => {
let apmFactoryMock

before(async () => {
apmFactoryMock = await APMRegistryFactoryMock.new(daoFactory.address, ...baseAddrs, '0x0', ensFactory.address)
apmFactoryMock = await APMRegistryFactoryMock.new(daoFactory.address, ...baseAddrs, ensFactory.address)
})

it('fails if factory doesnt give permission to create permissions', async () => {
return assertRevert(async () => {
await apmFactoryMock.newBadAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, true)
it('fails if factory doesnt give permission to create names', async () => {
await assertRevert(async () => {
await apmFactoryMock.newFailingAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, true)
})
})

it('fails if factory doesnt give permission to create names', async () => {
return assertRevert(async () => {
await apmFactoryMock.newBadAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, false)
it('fails if factory doesnt give permission to create permissions', async () => {
await assertRevert(async () => {
await apmFactoryMock.newFailingAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, false)
})
})
})
Expand Down
18 changes: 13 additions & 5 deletions test/ens_subdomains.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@ const Kernel = artifacts.require('Kernel')
const ACL = artifacts.require('ACL')

const APMRegistry = artifacts.require('APMRegistry')
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'

// Using APMFactory in order to reuse it
contract('ENSSubdomainRegistrar', accounts => {
let baseDeployed, apmFactory, ensFactory, daoFactory, ens, registrar
let baseDeployed, apmFactory, ensFactory, dao, daoFactory, ens, registrar
let APP_BASES_NAMESPACE

const ensOwner = accounts[0]
const apmOwner = accounts[1]
const repoDev = accounts[2]
Expand All @@ -38,6 +43,8 @@ contract('ENSSubdomainRegistrar', accounts => {
const kernelBase = await Kernel.new(true) // petrify immediately
const aclBase = await ACL.new()
daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, '0x00')

APP_BASES_NAMESPACE = await kernelBase.APP_BASES_NAMESPACE()
})

beforeEach(async () => {
Expand All @@ -49,7 +56,7 @@ contract('ENSSubdomainRegistrar', accounts => {
const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm
const registry = APMRegistry.at(apmAddr)

const dao = Kernel.at(await registry.kernel())
dao = Kernel.at(await registry.kernel())
const acl = ACL.at(await dao.acl())

registrar = ENSSubdomainRegistrar.at(await registry.registrar())
Expand Down Expand Up @@ -99,11 +106,12 @@ contract('ENSSubdomainRegistrar', accounts => {
})

it('fails if initializing without rootnode ownership', async () => {
const reg = await ENSSubdomainRegistrar.new()
const ens = await ENS.new()
const newRegProxy = await AppProxyUpgradeable.new(dao.address, namehash('apm-enssub.aragonpm.eth'), EMPTY_BYTES)
const newReg = ENSSubdomainRegistrar.at(newRegProxy.address)

return assertRevert(async () => {
await reg.initialize(ens.address, holanode)
await assertRevert(async () => {
await newReg.initialize(ens.address, holanode)
})
})
})
34 changes: 15 additions & 19 deletions test/evm_script.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ contract('EVM Script', accounts => {

assertEvent(receipt, 'DisableExecutor')
assert.isFalse(executorEntry[1], "executor should now be disabled")
assert.equal(await reg.getScriptExecutor(`0x0000000${newExecutorId}`), ZERO_ADDR, 'getting disabled executor should return zero addr')
})

it('can re-enable an executor', async () => {
Expand All @@ -117,12 +118,14 @@ contract('EVM Script', accounts => {
await reg.disableScriptExecutor(newExecutorId, { from: boss })
let executorEntry = await reg.executors(newExecutorId)
assert.isFalse(executorEntry[1], "executor should now be disabled")
assert.equal(await reg.getScriptExecutor(`0x0000000${newExecutorId}`), ZERO_ADDR, 'getting disabled executor should return zero addr')

const receipt = await reg.enableScriptExecutor(newExecutorId, { from: boss })
executorEntry = await reg.executors(newExecutorId)

assertEvent(receipt, 'EnableExecutor')
assert.isTrue(executorEntry[1], "executor should now be re-enabled")
assert.notEqual(await reg.getScriptExecutor(`0x0000000${newExecutorId}`), ZERO_ADDR, 'getting disabled executor should return non-zero addr')
})

it('fails to add a new executor without the correct permissions', async () => {
Expand Down Expand Up @@ -153,6 +156,13 @@ contract('EVM Script', accounts => {
})
})

it('fails to enable a non-existent executor', async () => {
await acl.createPermission(boss, reg.address, await reg.REGISTRY_MANAGER_ROLE(), boss, { from: boss })
await assertRevert(async () => {
await reg.enableScriptExecutor(newExecutorId + 1, { from: boss })
})
})

it('fails to disable an already disabled executor', async () => {
await acl.createPermission(boss, reg.address, await reg.REGISTRY_MANAGER_ROLE(), boss, { from: boss })
await reg.disableScriptExecutor(newExecutorId, { from: boss })
Expand Down Expand Up @@ -326,37 +336,23 @@ contract('EVM Script', accounts => {
})
})

context('registry', () => {
it('can be disabled', async () => {
context('> Registry actions', () => {
it("can't be executed once disabled", async () => {
await acl.createPermission(boss, reg.address, await reg.REGISTRY_MANAGER_ROLE(), boss, { from: boss })
const receipt = await reg.disableScriptExecutor(1, { from: boss })
const isEnabled = (await reg.executors(1))[1] // enabled flag is second in struct
await reg.disableScriptExecutor(1, { from: boss })

assertEvent(receipt, 'DisableExecutor')
assert.equal(await reg.getScriptExecutor('0x00000001'), ZERO_ADDR, 'getting disabled executor should return zero addr')
assert.isFalse(isEnabled, 'executor should be disabled')
return assertRevert(async () => {
await executorApp.execute(encodeCallScript([]))
})
})

it('can be re-enabled', async () => {
let isEnabled
await acl.createPermission(boss, reg.address, await reg.REGISTRY_MANAGER_ROLE(), boss, { from: boss })

// First, disable the executor
// Disable then re-enable the executor
await reg.disableScriptExecutor(1, { from: boss })
isEnabled = (await reg.executors(1))[1] // enabled flag is second in struct
assert.equal(await reg.getScriptExecutor('0x00000001'), ZERO_ADDR, 'getting disabled executor should return zero addr')
assert.isFalse(isEnabled, 'executor should be disabled')

// Then re-enable it
const receipt = await reg.enableScriptExecutor(1, { from: boss })
isEnabled = (await reg.executors(1))[1] // enabled flag is second in struct
await reg.enableScriptExecutor(1, { from: boss })

assertEvent(receipt, 'EnableExecutor')
assert.notEqual(await reg.getScriptExecutor('0x00000001'), ZERO_ADDR, 'getting enabled executor should be non-zero addr')
assert.isTrue(isEnabled, 'executor should be enabled')
await executorApp.execute(encodeCallScript([]))
})
})
Expand Down

0 comments on commit f8f5b67

Please sign in to comment.