Skip to content

Commit

Permalink
Split APM creation transaction
Browse files Browse the repository at this point in the history
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. The first one just creates the DAO, which currently consumes
~1.5M gas.
  • Loading branch information
ßingen committed Apr 18, 2018
1 parent 918126a commit eb218ef
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 11 deletions.
12 changes: 10 additions & 2 deletions contracts/factory/APMRegistryFactory.sol
Expand Up @@ -16,6 +16,7 @@ contract APMRegistryFactory is APMRegistryConstants {
ENSSubdomainRegistrar public ensSubdomainRegistrarBase;
ENS public ens;

event DeployAPMDao(bytes32 indexed node, address dao);
event DeployAPM(bytes32 indexed node, address apm);

// Needs either one ENS or ENSFactory
Expand All @@ -39,7 +40,14 @@ contract APMRegistryFactory is APMRegistryConstants {
ens = _ens != address(0) ? _ens : _ensFactory.newENS(this);
}

function newAPM(bytes32 _tld, bytes32 _label, address _root) public returns (APMRegistry) {
function newAPMDao(bytes32 _tld, bytes32 _label) public returns (Kernel) {
bytes32 node = keccak256(_tld, _label);
Kernel dao = daoFactory.newDAO(this);
DeployAPMDao(node, dao);
return dao;
}

function newAPM(bytes32 _tld, bytes32 _label, address _root, address _dao) public returns (APMRegistry) {
bytes32 node = keccak256(_tld, _label);

// Assume it is the test ENS
Expand All @@ -48,7 +56,7 @@ contract APMRegistryFactory is APMRegistryConstants {
ens.setSubnodeOwner(_tld, _label, this);
}

Kernel dao = daoFactory.newDAO(this);
Kernel dao = Kernel(_dao);
ACL acl = ACL(dao.acl());

acl.createPermission(this, dao, dao.APP_MANAGER_ROLE(), this);
Expand Down
8 changes: 6 additions & 2 deletions migrations/2_apm.js
@@ -1,6 +1,8 @@
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

Expand Down Expand Up @@ -30,8 +32,10 @@ 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 receiptDao = await factory.newAPMDao(namehash('eth'), '0x'+keccak256('aragonpm'))
const daoAddr = getEvent(receiptDao, 'DeployAPMDao', 'dao')
const receipt = await factory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), root, daoAddr)
const apmAddr = getEvent(receipt, 'DeployAPM', 'apm')
console.log('Deployed APM at:', apmAddr)

const apm = getContract('APMRegistry').at(apmAddr)
Expand Down
13 changes: 9 additions & 4 deletions test/apm_registry.js
Expand Up @@ -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 = {}
Expand Down Expand Up @@ -38,8 +39,10 @@ contract('APMRegistry', accounts => {
apmFactoryMock = await getContract('APMRegistryFactoryMock').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 receiptDao = await apmFactory.newAPMDao(namehash('eth'), '0x'+keccak256('aragonpm'))
const daoAddr = getEvent(receiptDao, 'DeployAPMDao', 'dao')
const receipt = await apmFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, daoAddr)
const apmAddr = getEvent(receipt, 'DeployAPM', 'apm')
registry = APMRegistry.at(apmAddr)

dao = Kernel.at(await registry.kernel())
Expand Down Expand Up @@ -84,8 +87,10 @@ contract('APMRegistry', accounts => {
const newFactory = await getContract('APMRegistryFactory').new(daoFactory.address, ...baseAddrs, ens2.address, '0x00')

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 receiptDao = await newFactory.newAPMDao(namehash('eth'), '0x'+keccak256('aragonpm'))
const daoAddr = getEvent(receiptDao, 'DeployAPMDao', 'dao')
const receipt2 = await newFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, daoAddr)
const apmAddr = getEvent(receipt2, 'DeployAPM', 'apm')
const resolver = PublicResolver.at(await ens2.resolver(rootNode))

assert.equal(await resolver.addr(rootNode), apmAddr, 'rootnode should be resolve')
Expand Down
7 changes: 5 additions & 2 deletions test/ens_subdomains.js
Expand Up @@ -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 => {
Expand Down Expand Up @@ -41,8 +42,10 @@ contract('ENSSubdomainRegistrar', accounts => {
const baseAddrs = baseDeployed.map(c => c.address)
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 receiptDao = await apmFactory.newAPMDao(namehash('eth'), '0x'+keccak256('aragonpm'))
const daoAddr = getEvent(receiptDao, 'DeployAPMDao', 'dao')
const receipt = await apmFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, daoAddr)
const apmAddr = getEvent(receipt, 'DeployAPM', 'apm')
registry = APMRegistry.at(apmAddr)

dao = Kernel.at(await registry.kernel())
Expand Down
2 changes: 1 addition & 1 deletion test/mocks/APMRegistryFactoryMock.sol
Expand Up @@ -16,7 +16,7 @@ contract APMRegistryFactoryMock is APMRegistryFactory {
)
APMRegistryFactory(_daoFactory, _registryBase, _repoBase, _ensSubBase, _ens, _ensFactory) public {}

function newAPM(bytes32 tld, bytes32 label, address _root) public returns (APMRegistry) {}
function newAPM(bytes32 tld, bytes32 label, address _root, address _dao) public returns (APMRegistry) {}

function newBadAPM(bytes32 tld, bytes32 label, address _root, bool withoutACL) public returns (APMRegistry) {
bytes32 node = keccak256(tld, label);
Expand Down

0 comments on commit eb218ef

Please sign in to comment.