Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimizes app installations #125

Merged
merged 1 commit into from Aug 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 18 additions & 14 deletions shared/contracts/BaseTemplate.sol
Expand Up @@ -32,6 +32,11 @@ contract BaseTemplate is APMNamehash, IsContract {
bytes32 constant internal FINANCE_APP_ID = 0xbf8491150dafc5dcaee5b861414dca922de09ccffa344964ae167212e8c673ae;
bytes32 constant internal TOKEN_MANAGER_APP_ID = 0x6b20a3010614eeebf2138ccec99f028a61c811b3b1a3343b6ff635985c75c91f;

bytes4 constant private AGENT_INIT_SELECTOR = 0x8129fc1c;
bytes4 constant private VAULT_INIT_SELECTOR = 0x8129fc1c;
bytes4 constant private FINANCE_INIT_SELECTOR = 0x1798de81;
bytes4 constant private VOTING_INIT_SELECTOR = 0xdf3d3305;
facuspagnuolo marked this conversation as resolved.
Show resolved Hide resolved

string constant private ERROR_ENS_NOT_CONTRACT = "TEMPLATE_ENS_NOT_CONTRACT";
string constant private ERROR_DAO_FACTORY_NOT_CONTRACT = "TEMPLATE_DAO_FAC_NOT_CONTRACT";
string constant private ERROR_ARAGON_ID_NOT_PROVIDED = "TEMPLATE_ARAGON_ID_NOT_PROVIDED";
Expand Down Expand Up @@ -109,18 +114,15 @@ contract BaseTemplate is APMNamehash, IsContract {
/* AGENT */

function _installDefaultAgentApp(Kernel _dao) internal returns (Agent) {
Agent agent = Agent(_installDefaultApp(_dao, AGENT_APP_ID));
agent.initialize();
Agent agent = Agent(_installDefaultApp(_dao, AGENT_APP_ID, abi.encodeWithSelector(AGENT_INIT_SELECTOR)));
// We assume that installing the Agent app as default is in order to replace the Vault app which is
// normally installed as default. Thus, we are setting its ID as the Vault id that the Kernel will use.
_dao.setRecoveryVaultAppId(AGENT_APP_ID);
return agent;
}

function _installNonDefaultAgentApp(Kernel _dao) internal returns (Agent) {
Agent agent = Agent(_installNonDefaultApp(_dao, AGENT_APP_ID));
agent.initialize();
return agent;
return Agent(_installNonDefaultApp(_dao, AGENT_APP_ID, abi.encodeWithSelector(AGENT_INIT_SELECTOR)));
}

function _createAgentPermissions(ACL _acl, Agent _agent, address _grantee, address _manager) internal {
Expand All @@ -131,9 +133,7 @@ contract BaseTemplate is APMNamehash, IsContract {
/* FINANCE */

function _installFinanceApp(Kernel _dao, Vault _vault, uint64 _periodDuration) internal returns (Finance) {
Finance finance = Finance(_installNonDefaultApp(_dao, FINANCE_APP_ID));
finance.initialize(_vault, _periodDuration);
return finance;
return Finance(_installNonDefaultApp(_dao, FINANCE_APP_ID, abi.encodeWithSelector(FINANCE_INIT_SELECTOR, _vault, _periodDuration)));
}

function _createFinancePermissions(ACL _acl, Finance _finance, address _grantee, address _manager) internal {
Expand Down Expand Up @@ -182,9 +182,7 @@ contract BaseTemplate is APMNamehash, IsContract {
/* VAULT */

function _installVaultApp(Kernel _dao) internal returns (Vault) {
Vault vault = Vault(_installDefaultApp(_dao, VAULT_APP_ID));
vault.initialize();
return vault;
return Vault(_installDefaultApp(_dao, VAULT_APP_ID, abi.encodeWithSelector(VAULT_INIT_SELECTOR)));
}

function _createVaultPermissions(ACL _acl, Vault _vault, address _grantee, address _manager) internal {
Expand All @@ -194,9 +192,7 @@ contract BaseTemplate is APMNamehash, IsContract {
/* VOTING */

function _installVotingApp(Kernel _dao, MiniMeToken _token, uint64 _support, uint64 _acceptance, uint64 _duration) internal returns (Voting) {
Voting voting = Voting(_installNonDefaultApp(_dao, VOTING_APP_ID));
voting.initialize(_token, _support, _acceptance, _duration);
return voting;
return Voting(_installNonDefaultApp(_dao, VOTING_APP_ID, abi.encodeWithSelector(VOTING_INIT_SELECTOR, _token, _support, _acceptance, _duration)));
}

function _createVotingPermissions(ACL _acl, Voting _voting, address _grantee, address _manager) internal {
Expand All @@ -218,10 +214,18 @@ contract BaseTemplate is APMNamehash, IsContract {
return _installApp(_dao, _appId, new bytes(0), false);
}

function _installNonDefaultApp(Kernel _dao, bytes32 _appId, bytes _data) internal returns (address) {
return _installApp(_dao, _appId, _data, false);
}

function _installDefaultApp(Kernel _dao, bytes32 _appId) internal returns (address) {
return _installApp(_dao, _appId, new bytes(0), true);
}

function _installDefaultApp(Kernel _dao, bytes32 _appId, bytes _data) internal returns (address) {
return _installApp(_dao, _appId, _data, true);
}

function _installApp(Kernel _dao, bytes32 _appId, bytes _data, bool _setDefault) internal returns (address) {
address latestBaseAppAddress = _latestVersionAppBase(_appId);
address instance = address(_dao.newAppInstance(_appId, latestBaseAppAddress, _data, _setDefault));
Expand Down
8 changes: 4 additions & 4 deletions templates/company-board/test/company-board.js
Expand Up @@ -183,13 +183,13 @@ contract('Company with board', ([_, owner, boardMember1, boardMember2, shareHold
shareTokenManager = TokenManager.at(installedApps['token-manager'][1])
})

it('costs ~10.4e6 gas', async () => {
it('costs max ~10.29e6 gas', async () => {
facuspagnuolo marked this conversation as resolved.
Show resolved Hide resolved
const prepareGas = prepareReceipt.receipt.gasUsed
const setupGas = setupReceipt.receipt.gasUsed
const totalGas = prepareGas + setupGas
assert.isAtMost(prepareGas, 5.0e6, 'prepare script should cost almost 5.0e6 gas')
assert.isAtMost(setupGas, 5.4e6, 'setup script should cost almost 5.4e6 gas')
assert.isAtMost(totalGas, 10.4e6, 'prepare + setup scripts should cost almost 10.4e6 gas')
assert.isAtMost(prepareGas, 4.99e6, 'prepare script should cost almost 4.99e6 gas')
assert.isAtMost(setupGas, 5.30e6, 'setup script should cost almost 5.30e6 gas')
assert.isAtMost(totalGas, 10.29e6, 'prepare + setup scripts should cost almost 10.29e6 gas')
})

it('registers a new DAO on ENS', async () => {
Expand Down
8 changes: 4 additions & 4 deletions templates/company/test/company.js
Expand Up @@ -136,12 +136,12 @@ contract('Company', ([_, owner, holder1, holder2]) => {
tokenManager = TokenManager.at(installedApps['token-manager'][0])
})

it('costs ~6.9e6 gas', async () => {
it('costs max ~6.73e6 gas', async () => {
if (creationStyle === 'single') {
assert.isAtMost(instanceReceipt.receipt.gasUsed, 6.8e6, 'create script should cost almost 6.8e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 6.71e6, 'create script should cost almost 6.71e6 gas')
} else if (creationStyle === 'separate') {
assert.isAtMost(tokenReceipt.receipt.gasUsed, 1.8e6, 'create token script should cost almost 1.8e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 5.1e6, 'create instance script should cost almost 5e6 gas')
assert.isAtMost(tokenReceipt.receipt.gasUsed, 1.74e6, 'create token script should cost almost 1.74e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 4.99e6, 'create instance script should cost almost 4.99e6 gas')
}
})

Expand Down
8 changes: 4 additions & 4 deletions templates/membership/test/membership.js
Expand Up @@ -125,12 +125,12 @@ contract('Membership', ([_, owner, member1, member2]) => {
tokenManager = TokenManager.at(installedApps['token-manager'][0])
})

it('costs ~6.9e6 gas', async () => {
it('costs max ~6.73e6 gas', async () => {
if (creationStyle === 'single') {
assert.isAtMost(instanceReceipt.receipt.gasUsed, 6.8e6, 'create script should cost almost 6.8e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 6.71e6, 'create script should cost almost 6.71e6 gas')
} else if (creationStyle === 'separate') {
assert.isAtMost(tokenReceipt.receipt.gasUsed, 1.8e6, 'create token script should cost almost 1.8e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 5.1e6, 'create instance script should cost almost 5.1e6 gas')
assert.isAtMost(tokenReceipt.receipt.gasUsed, 1.74e6, 'create token script should cost almost 1.74e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 5.00e6, 'create instance script should cost almost 5.00e6 gas')
}
})

Expand Down
8 changes: 4 additions & 4 deletions templates/reputation/test/reputation.js
Expand Up @@ -135,12 +135,12 @@ contract('Reputation', ([_, owner, holder1, holder2]) => {
tokenManager = TokenManager.at(installedApps['token-manager'][0])
})

it('costs ~7e6 gas', async () => {
it('costs max ~6.74e6 gas', async () => {
if (creationStyle === 'single') {
assert.isAtMost(instanceReceipt.receipt.gasUsed, 6.8e6, 'create script should cost almost 6.8e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 6.72e6, 'create script should cost almost 6.72e6 gas')
} else if (creationStyle === 'separate') {
assert.isAtMost(tokenReceipt.receipt.gasUsed, 1.8e6, 'create token script should cost almost 1.8e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 5.2e6, 'create instance script should cost almost 5.2e6 gas')
assert.isAtMost(tokenReceipt.receipt.gasUsed, 1.74e6, 'create token script should cost almost 1.74e6 gas')
assert.isAtMost(instanceReceipt.receipt.gasUsed, 5.00e6, 'create instance script should cost almost 5.00e6 gas')
}
})

Expand Down
8 changes: 4 additions & 4 deletions templates/trust/test/trust.js
Expand Up @@ -118,10 +118,10 @@ contract('Trust', ([_, owner, beneficiaryKey1, beneficiaryKey2, heir1, heir2, mu
heirsTokenManager = TokenManager.at(installedApps['token-manager'][1])
})

it('costs ~13e6 gas', async () => {
assert.isAtMost(prepareReceipt.receipt.gasUsed, 5e6, 'prepare script should cost almost 5e6 gas')
assert.isAtMost(setupReceipt.receipt.gasUsed, 6e6, 'setup script should cost almost 6e6 gas')
assert.isAtMost(multiSigSetupReceipt.receipt.gasUsed, 2e6, 'multisig script should cost almost 2e6 gas')
it('costs ~12.51e6 gas', async () => {
assert.isAtMost(prepareReceipt.receipt.gasUsed, 5.00e6, 'prepare script should cost almost 5.00e6 gas')
assert.isAtMost(setupReceipt.receipt.gasUsed, 5.67e6, 'setup script should cost almost 5.67e6 gas')
assert.isAtMost(multiSigSetupReceipt.receipt.gasUsed, 1.84e6, 'multisig script should cost almost 1.84e6 gas')
})

it('registers a new DAO on ENS', async () => {
Expand Down