Split APM creation transaction.

To fix gas issues and on deployment and `npm run test`, `newAPM` is
now split into 2 transactions to avoid passing the transaction gas
limit.

On deployment of new APM we must be careful because if only the second
transaction fails the APM would be created but would have wrong
permissions.

contracts/factory/APMRegistryFactory.sol

@@ -16,7 +16,7 @@ contract APMRegistryFactory is APMRegistryConstants {
     ENSSubdomainRegistrar public ensSubdomainRegistrarBase;
     ENS public ens;
 
-    event DeployAPM(bytes32 indexed node, address apm);
+    event DeployAPM(bytes32 indexed node, address apm, address acl);
 
     // Needs either one ENS or ENSFactory
     function APMRegistryFactory(
@@ -62,22 +62,27 @@ contract APMRegistryFactory is APMRegistryConstants {
         // APMRegistry controls Repos
         dao.setApp(namespace, keccak256(node, keccak256(REPO_APP_NAME)), repoBase);
 
-        DeployAPM(node, apm);
+        DeployAPM(node, apm, acl);
 
         // Grant permissions needed for APM on ENSSubdomainRegistrar
         acl.createPermission(apm, ensSub, ensSub.CREATE_NAME_ROLE(), _root);
         acl.createPermission(apm, ensSub, ensSub.POINT_ROOTNODE_ROLE(), _root);
 
         // allow apm to create permissions for Repos in Kernel
-        bytes32 permRole = acl.CREATE_PERMISSIONS_ROLE();
-
-        acl.grantPermission(apm, acl, permRole);
+        acl.grantPermission(apm, acl, acl.CREATE_PERMISSIONS_ROLE());
 
         // Initialize
         ens.setOwner(node, ensSub);
         ensSub.initialize(ens, node);
         apm.initialize(ensSub);
 
+        // Permission transition to _root
+        acl.setPermissionManager(_root, dao, dao.APP_MANAGER_ROLE());
+
+        return apm;
+    }
+
+    function createRepos(APMRegistry apm, ACL acl, address _root) public {
         uint16[3] memory firstVersion;
         firstVersion[0] = 1;
 
@@ -90,12 +95,10 @@ contract APMRegistryFactory is APMRegistryConstants {
         configureAPMPermissions(acl, apm, _root);
 
         // Permission transition to _root
-        acl.setPermissionManager(_root, dao, dao.APP_MANAGER_ROLE());
+        bytes32 permRole = acl.CREATE_PERMISSIONS_ROLE();
         acl.revokePermission(this, acl, permRole);
         acl.grantPermission(_root, acl, permRole);
         acl.setPermissionManager(_root, acl, permRole);
-
-        return apm;
     }
 
     function b(string memory x) internal pure returns (bytes memory y) {

migrations/2_apm.js

@@ -1,5 +1,6 @@
 const namehash = require('eth-ens-namehash').hash
 const keccak256 = require('js-sha3').keccak_256
+const getEvent = (receipt, event, arg) => { return receipt.logs.filter(l => l.event == event)[0].args[arg] }
 
 module.exports = async (deployer, network, accounts, arts = null) => {
   if (arts != null) artifacts = arts // allow running outside
@@ -31,7 +32,9 @@ module.exports = async (deployer, network, accounts, arts = null) => {
   console.log('Deploying APM...')
   const root = '0xffffffffffffffffffffffffffffffffffffffff' // public
   const receipt = await factory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), root)
-  const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm
+  const apmAddr = getEvent(receipt, 'DeployAPM', 'apm')
+  const aclAddr = getEvent(receipt, 'DeployAPM', 'acl')
+  await factory.createRepos(apmAddr, aclAddr, root)
   console.log('Deployed APM at:', apmAddr)
 
   const apm = getContract('APMRegistry').at(apmAddr)

test/apm_registry.js

@@ -10,6 +10,7 @@ const Kernel = artifacts.require('Kernel')
 const ACL = artifacts.require('ACL')
 
 const getContract = name => artifacts.require(name)
+const getEvent = (receipt, event, arg) => { return receipt.logs.filter(l => l.event == event)[0].args[arg] }
 
 contract('APMRegistry', accounts => {
     let ensFactory, ens, apmFactory, apmFactoryMock, registry, baseDeployed, baseAddrs, dao, acl, daoFactory = {}
@@ -39,7 +40,9 @@ contract('APMRegistry', accounts => {
         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 apmAddr = getEvent(receipt, 'DeployAPM', 'apm')
+        const aclAddr = getEvent(receipt, 'DeployAPM', 'acl')
+        await apmFactory.createRepos(apmAddr, aclAddr, apmOwner)
         registry = APMRegistry.at(apmAddr)
 
         dao = Kernel.at(await registry.kernel())
@@ -85,7 +88,9 @@ contract('APMRegistry', accounts => {
 
         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
+        const apmAddr = getEvent(receipt2, 'DeployAPM', 'apm')
+        const aclAddr = getEvent(receipt2, 'DeployAPM', 'acl')
+        await newFactory.createRepos(apmAddr, aclAddr, apmOwner)
         const resolver = PublicResolver.at(await ens2.resolver(rootNode))
 
         assert.equal(await resolver.addr(rootNode), apmAddr, 'rootnode should be resolve')

test/ens_subdomains.js

@@ -11,6 +11,7 @@ const ACL = artifacts.require('ACL')
 const ENSSubdomainRegistrar = artifacts.require('ENSSubdomainRegistrar')
 
 const getContract = name => artifacts.require(name)
+const getEvent = (receipt, event, arg) => { return receipt.logs.filter(l => l.event == event)[0].args[arg] }
 
 // Using APMFactory in order to reuse it
 contract('ENSSubdomainRegistrar', accounts => {
@@ -42,7 +43,9 @@ contract('ENSSubdomainRegistrar', accounts => {
         apmFactory = await getContract('APMRegistryFactory').new(daoFactory.address, ...baseAddrs, '0x0', 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 apmAddr = getEvent(receipt, 'DeployAPM', 'apm')
+        const aclAddr = getEvent(receipt, 'DeployAPM', 'acl')
+        await apmFactory.createRepos(apmAddr, aclAddr, apmOwner)
         registry = APMRegistry.at(apmAddr)
 
         dao = Kernel.at(await registry.kernel())

test/mocks/APMRegistryFactoryMock.sol

@@ -39,7 +39,7 @@ contract APMRegistryFactoryMock is APMRegistryFactory {
         // APMRegistry controls Repos
         dao.setApp(namespace, keccak256(node, REPO_APP_NAME), repoBase);
 
-        DeployAPM(node, apm);
+        DeployAPM(node, apm, acl);
 
         // Grant permissions needed for APM on ENSSubdomainRegistrar