Skip to content

Commit

Permalink
Merge f67223b into 7df3578
Browse files Browse the repository at this point in the history
  • Loading branch information
sohkai committed Mar 19, 2019
2 parents 7df3578 + f67223b commit aad0e9d
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 66 deletions.
31 changes: 17 additions & 14 deletions test/apm_registry.js
Expand Up @@ -19,6 +19,9 @@ 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 => {
let baseDeployed, baseAddrs, ensFactory, apmFactory, daoFactory, ens, registry, acl
const ensOwner = accounts[0]
Expand All @@ -38,11 +41,11 @@ contract('APMRegistry', accounts => {

const kernelBase = await Kernel.new(true) // petrify immediately
const aclBase = await ACL.new()
daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, '0x00')
daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, ZERO_ADDR)
})

beforeEach(async () => {
apmFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, '0x0', ensFactory.address)
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)
Expand All @@ -60,7 +63,7 @@ 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 newFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ens2.address, '0x00')
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)
Expand All @@ -81,20 +84,20 @@ 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], '0x00', '0x00', { from: apmOwner })
const receipt = await registry.newRepoWithVersion('test', repoDev, [1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: apmOwner })
const repo = Repo.at(getRepoFromLog(receipt))

assert.equal(await repo.getVersionsCount(), 1, 'should have created version')

await repo.newVersion([2, 0, 0], '0x00', '0x00', { from: repoDev })
await repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })

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

// Factory doesn't have ownership over 'eth' tld
await assertRevert(async () => {
Expand Down Expand Up @@ -141,36 +144,36 @@ contract('APMRegistry', accounts => {
})

it('repo dev can create versions', async () => {
await repo.newVersion([1, 0, 0], '0x00', '0x00', { from: repoDev })
await repo.newVersion([2, 0, 0], '0x00', '0x00', { from: repoDev })
await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })
await repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })

assert.equal(await repo.getVersionsCount(), 2, 'should have created versions')
})

it('repo dev can authorize someone to interact with repo', async () => {
await repo.newVersion([1, 0, 0], '0x00', '0x00', { from: repoDev })
await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })
const newOwner = accounts[8]

await acl.grantPermission(newOwner, repo.address, await repo.CREATE_VERSION_ROLE(), { from: repoDev })

await repo.newVersion([2, 0, 0], '0x00', '0x00', { from: newOwner })
await repo.newVersion([2, 1, 0], '0x00', '0x00', { from: repoDev }) // repoDev can still create them
await repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: newOwner })
await repo.newVersion([2, 1, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev }) // repoDev can still create them

assert.equal(await repo.getVersionsCount(), 3, 'should have created versions')
})

it('repo dev can no longer create versions if permission is removed', async () => {
await repo.newVersion([1, 0, 0], '0x00', '0x00', { from: repoDev })
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], '0x00', '0x00', { from: repoDev })
await 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], '0x00', '0x00', { from: notOwner })
await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: notOwner })
})
})
})
Expand Down
8 changes: 5 additions & 3 deletions test/apm_repo.js
@@ -1,8 +1,10 @@
const { assertRevert } = require('./helpers/assertThrow')


const Repo = artifacts.require('UnsafeRepo')

const EMPTY_BYTES = '0x'
const ZERO_ADDR = '0x0000000000000000000000000000000000000000'

contract('Repo', accounts => {
let repo

Expand Down Expand Up @@ -31,7 +33,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], '0x00', '0x00')
await repo.newVersion([1, 1, 0], ZERO_ADDR, EMPTY_BYTES)
})
})

Expand Down Expand Up @@ -71,7 +73,7 @@ contract('Repo', accounts => {
})

it('setting contract address to 0 reuses last version address', async () => {
await repo.newVersion([1, 1, 0], '0x00', initialContent)
await repo.newVersion([1, 1, 0], ZERO_ADDR, initialContent)
assertVersion(await repo.getByVersionId(2), [1, 1, 0], initialCode, initialContent)
})

Expand Down
11 changes: 5 additions & 6 deletions test/ens_subdomains.js
Expand Up @@ -17,6 +17,7 @@ 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 => {
Expand All @@ -32,8 +33,6 @@ contract('ENSSubdomainRegistrar', accounts => {
const holanode = namehash('hola.aragonpm.eth')
const holalabel = '0x'+keccak256('hola')

const zeroAddr = '0x0000000000000000000000000000000000000000'

before(async () => {
const bases = [APMRegistry, Repo, ENSSubdomainRegistrar]
baseDeployed = await Promise.all(bases.map(base => base.new()))
Expand All @@ -42,14 +41,14 @@ 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')
daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, ZERO_ADDR)

APP_BASES_NAMESPACE = await kernelBase.APP_BASES_NAMESPACE()
})

beforeEach(async () => {
const baseAddrs = baseDeployed.map(c => c.address)
apmFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, '0x0', ensFactory.address)
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)
Expand Down Expand Up @@ -95,14 +94,14 @@ contract('ENSSubdomainRegistrar', accounts => {
await registrar.createName(holalabel, apmOwner, { from: apmOwner })
await registrar.deleteName(holalabel, { from: apmOwner })

assert.equal(await ens.owner(holanode), zeroAddr, 'should have reset name')
assert.equal(await ens.owner(holanode), ZERO_ADDR, 'should have reset name')
})

it('can delete names registered to itself', async () => {
await registrar.createName(holalabel, registrar.address, { from: apmOwner })
await registrar.deleteName(holalabel, { from: apmOwner })

assert.equal(await ens.owner(holanode), zeroAddr, 'should have reset name')
assert.equal(await ens.owner(holanode), ZERO_ADDR, 'should have reset name')
})

it('fails if initializing without rootnode ownership', async () => {
Expand Down
41 changes: 22 additions & 19 deletions test/evm_script.js
Expand Up @@ -18,6 +18,7 @@ const ExecutionTarget = artifacts.require('ExecutionTarget')
const EVMScriptExecutorMock = artifacts.require('EVMScriptExecutorMock')
const MockScriptExecutorApp = artifacts.require('MockScriptExecutorApp')

const EMPTY_BYTES = '0x'
const ZERO_ADDR = '0x0000000000000000000000000000000000000000'

contract('EVM Script', accounts => {
Expand All @@ -28,6 +29,8 @@ contract('EVM Script', accounts => {

const executorAppId = '0x1234'

const createExecutorId = id => `0x${String(id).padStart(8, '0')}`

before(async () => {
const regFact = await EVMScriptRegistryFactory.new()
callsScriptBase = CallsScript.at(await regFact.baseCallScript())
Expand All @@ -51,17 +54,17 @@ contract('EVM Script', accounts => {
})

it('factory registered just 1 script executor', async () => {
assert.equal(await reg.getScriptExecutor('0x00000000'), ZERO_ADDR)
assert.notEqual(await reg.getScriptExecutor('0x00000001'), ZERO_ADDR)
assert.equal(await reg.getScriptExecutor('0x00000002'), ZERO_ADDR)
assert.equal(await reg.getScriptExecutor(createExecutorId(0)), ZERO_ADDR)
assert.notEqual(await reg.getScriptExecutor(createExecutorId(1)), ZERO_ADDR)
assert.equal(await reg.getScriptExecutor(createExecutorId(2)), ZERO_ADDR)
})

it('fails if directly calling base executor', async () => {
const executionTarget = await ExecutionTarget.new()
const action = { to: executionTarget.address, calldata: executionTarget.contract.execute.getData() }
const script = encodeCallScript([action])

await assertRevert(() => callsScriptBase.execScript(script, '0x', []))
await assertRevert(() => callsScriptBase.execScript(script, EMPTY_BYTES, []))
})

context('> Registry', () => {
Expand Down Expand Up @@ -109,7 +112,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')
assert.equal(await reg.getScriptExecutor(createExecutorId(newExecutorId)), ZERO_ADDR, 'getting disabled executor should return zero addr')
})

it('can re-enable an executor', async () => {
Expand All @@ -118,14 +121,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')
assert.equal(await reg.getScriptExecutor(createExecutorId(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')
assert.notEqual(await reg.getScriptExecutor(createExecutorId(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 @@ -176,7 +179,7 @@ contract('EVM Script', accounts => {

context('> Uninitialized executor', () => {
beforeEach(async () => {
const receipt = await dao.newAppInstance(executorAppId, executorAppBase.address, '0x', false, { from: boss })
const receipt = await dao.newAppInstance(executorAppId, executorAppBase.address, EMPTY_BYTES, false, { from: boss })
executorApp = MockScriptExecutorApp.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
// Explicitly don't initialize the executorApp
executionTarget = await ExecutionTarget.new()
Expand All @@ -192,7 +195,7 @@ contract('EVM Script', accounts => {

context('> Executor', () => {
beforeEach(async () => {
const receipt = await dao.newAppInstance(executorAppId, executorAppBase.address, '0x', false, { from: boss })
const receipt = await dao.newAppInstance(executorAppId, executorAppBase.address, EMPTY_BYTES, false, { from: boss })
executorApp = MockScriptExecutorApp.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
await executorApp.initialize()
executionTarget = await ExecutionTarget.new()
Expand All @@ -205,29 +208,29 @@ contract('EVM Script', accounts => {

it('fails to execute if spec ID is 0', async () => {
return assertRevert(async () => {
await executorApp.execute('0x00000000')
await executorApp.execute(createExecutorId(0))
})
})

it('fails to execute if spec ID is unknown', async () => {
return assertRevert(async () => {
await executorApp.execute('0x00000004')
await executorApp.execute(createExecutorId(4))
})
})

context('> Spec ID 1', () => {
it('is the correct executor type', async () => {
const CALLS_SCRIPT_TYPE = soliditySha3('CALLS_SCRIPT')
const executor = IEVMScriptExecutor.at(await reg.getScriptExecutor('0x00000001'))
const executor = IEVMScriptExecutor.at(await reg.getScriptExecutor(createExecutorId(1)))
assert.equal(await executor.executorType(), CALLS_SCRIPT_TYPE)
})

it('gets the correct executor from the app', async () => {
const CALLS_SCRIPT_TYPE = soliditySha3('CALLS_SCRIPT')
const script = '0x00000001'
const executor = await reg.getScriptExecutor(script)
const scriptId = createExecutorId(1)
const executor = await reg.getScriptExecutor(scriptId)

const appExecutor = await executorApp.getEVMScriptExecutor(script)
const appExecutor = await executorApp.getEVMScriptExecutor(scriptId)
assert.equal(executor, appExecutor, 'app should return the same evm script executor')
})

Expand All @@ -242,10 +245,10 @@ contract('EVM Script', accounts => {
// 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 reg.getScriptExecutor('0x00000001'), 'should log the same executor')
assert.equal(scriptResult.args.executor, await reg.getScriptExecutor(createExecutorId(1)), 'should log the same executor')
assert.equal(scriptResult.args.script, script, 'should log the same script')
assert.equal(scriptResult.args.input, '0x', 'should log the same input')
assert.equal(scriptResult.args.returnData, '0x', 'should log the return data')
assert.equal(scriptResult.args.input, EMPTY_BYTES, 'should log the same input')
assert.equal(scriptResult.args.returnData, EMPTY_BYTES, 'should log the return data')
})

it('fails to execute if has blacklist addresses being called', async () => {
Expand Down Expand Up @@ -323,7 +326,7 @@ contract('EVM Script', accounts => {

// Remove 12 first 0s of padding for addr and 28 0s for uint32
return script + addr.slice(24) + length.slice(56) + calldata.slice(2)
}, '0x00000001') // spec 1
}, createExecutorId(1)) // spec 1
}

it('fails if data length is too big', async () => {
Expand Down

0 comments on commit aad0e9d

Please sign in to comment.