Skip to content

Commit

Permalink
Kernel: use explicit mapping for namespaces rather than hash (#410)
Browse files Browse the repository at this point in the history
Fixes #337.

I've opted to keep the "app identifier" nomenclature rather than "app name" due to it breaking less external interfaces (e.g. events). A name and an identifier are pretty similar anyway, we just shouldn't have both concepts.
  • Loading branch information
sohkai committed Sep 3, 2018
1 parent f6cd59b commit 5b880fc
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 121 deletions.
4 changes: 2 additions & 2 deletions contracts/apps/AppProxyBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ pragma solidity 0.4.24;

import "./AppStorage.sol";
import "../common/DepositableDelegateProxy.sol";
import "../kernel/KernelStorage.sol";
import "../kernel/KernelConstants.sol";
import "../kernel/IKernel.sol";


Expand Down Expand Up @@ -33,6 +33,6 @@ contract AppProxyBase is AppStorage, DepositableDelegateProxy, KernelConstants {
}

function getAppBase(bytes32 _appId) internal view returns (address) {
return kernel().getApp(keccak256(abi.encodePacked(APP_BASES_NAMESPACE, _appId)));
return kernel().getApp(APP_BASES_NAMESPACE, _appId);
}
}
5 changes: 3 additions & 2 deletions contracts/evmscript/EVMScriptRunner.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import "./IEVMScriptExecutor.sol";
import "./IEVMScriptRegistry.sol";

import "../apps/AppStorage.sol";
import "../kernel/KernelConstants.sol";
import "../common/Initializable.sol";


contract EVMScriptRunner is AppStorage, Initializable, EVMScriptRegistryConstants {
contract EVMScriptRunner is AppStorage, Initializable, EVMScriptRegistryConstants, KernelConstants {
event ScriptResult(address indexed executor, bytes script, bytes input, bytes returnData);

function getExecutor(bytes _script) public view returns (IEVMScriptExecutor) {
Expand Down Expand Up @@ -40,7 +41,7 @@ contract EVMScriptRunner is AppStorage, Initializable, EVMScriptRegistryConstant
}

function getExecutorRegistry() internal view returns (IEVMScriptRegistry) {
address registryAddr = kernel().getApp(EVMSCRIPT_REGISTRY_APP);
address registryAddr = kernel().getApp(APP_ADDR_NAMESPACE, EVMSCRIPT_REGISTRY_APP_ID);
return IEVMScriptRegistry(registryAddr);
}

Expand Down
4 changes: 0 additions & 4 deletions contracts/evmscript/IEVMScriptRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,9 @@ import "./IEVMScriptExecutor.sol";

contract EVMScriptRegistryConstants {
/* Hardcoded constants to save gas
bytes32 constant public APP_ADDR_NAMESPACE = keccak256("app");
bytes32 constant public EVMSCRIPT_REGISTRY_APP_ID = apmNamehash("evmreg");
bytes32 constant public EVMSCRIPT_REGISTRY_APP = keccak256(APP_ADDR_NAMESPACE, EVMSCRIPT_REGISTRY_APP_ID);
*/
bytes32 constant public EVMSCRIPT_REGISTRY_APP_ID = 0xddbcfd564f642ab5627cf68b9b7d374fb4f8a36e941a75d89c87998cef03bd61;
bytes32 constant public EVMSCRIPT_REGISTRY_APP = 0x34f01c17e9be6ddbf2c61f37b5b1fb9f1a090a975006581ad19bda1c4d382871;
}


Expand Down
6 changes: 3 additions & 3 deletions contracts/kernel/IKernel.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import "../common/IVaultRecoverable.sol";

// This should be an interface, but interfaces can't inherit yet :(
contract IKernel is IVaultRecoverable {
event SetApp(bytes32 indexed namespace, bytes32 indexed name, bytes32 indexed id, address app);
event SetApp(bytes32 indexed namespace, bytes32 indexed appId, address app);

function acl() public view returns (IACL);
function hasPermission(address who, address where, bytes32 what, bytes how) public view returns (bool);

function setApp(bytes32 namespace, bytes32 name, address app) public returns (bytes32 id);
function getApp(bytes32 id) public view returns (address);
function setApp(bytes32 namespace, bytes32 appId, address app) public;
function getApp(bytes32 namespace, bytes32 appId) public view returns (address);
}
103 changes: 51 additions & 52 deletions contracts/kernel/Kernel.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import "../factory/AppProxyFactory.sol";
contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecoverable, AppProxyFactory, ACLSyntaxSugar {
// Hardcode constant to save gas
//bytes32 constant public APP_MANAGER_ROLE = keccak256("APP_MANAGER_ROLE");
//bytes32 constant public DEFAULT_VAULT_ID = keccak256(APP_ADDR_NAMESPACE, apmNamehash("vault"));
//bytes32 constant public DEFAULT_VAULT_APP_ID = apmNamehash("vault");
bytes32 constant public APP_MANAGER_ROLE = 0xb6d92708f3d4817afc106147d969e229ced5c46e65e0a5002a0d391287762bd0;
bytes32 constant public DEFAULT_VAULT_ID = 0x4214e5fd6d0170d69ea641b5614f5093ebecc9928af51e95685c87617489800e;
bytes32 constant public DEFAULT_VAULT_APP_ID = 0x7e852e0fcfce6551c13800f1e7476f982525c2b5277ba14b24339c68416336d1;

/**
* @dev Constructor that allows the deployer to choose if the base instance should be petrified immediately.
Expand All @@ -37,138 +37,142 @@ contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecover
function initialize(IACL _baseAcl, address _permissionsCreator) public onlyInit {
initialized();

IACL acl = IACL(newAppProxy(this, ACL_APP_ID));

// Set ACL base
_setApp(APP_BASES_NAMESPACE, ACL_APP_ID, _baseAcl);
_setApp(APP_ADDR_NAMESPACE, ACL_APP_ID, acl);

// Create ACL instance and attach it as the default ACL app
IACL acl = IACL(newAppProxy(this, ACL_APP_ID));
acl.initialize(_permissionsCreator);
_setApp(APP_ADDR_NAMESPACE, ACL_APP_ID, acl);

recoveryVaultId = DEFAULT_VAULT_ID;
recoveryVaultAppId = DEFAULT_VAULT_APP_ID;
}

/**
* @dev Create a new instance of an app linked to this kernel
* @param _name Name of the app
* @param _appId Identifier for app
* @param _appBase Address of the app's base implementation
* @return AppProxy instance
*/
function newAppInstance(bytes32 _name, address _appBase)
function newAppInstance(bytes32 _appId, address _appBase)
public
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _name))
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _appId))
returns (ERCProxy appProxy)
{
return newAppInstance(_name, _appBase, false);
return newAppInstance(_appId, _appBase, false);
}

/**
* @dev Create a new instance of an app linked to this kernel and set its base
* implementation if it was not already set
* @param _name Name of the app
* @param _appId Identifier for app
* @param _appBase Address of the app's base implementation
* @param _setDefault Whether the app proxy app is the default one.
* Useful when the Kernel needs to know of an instance of a particular app,
* like Vault for escape hatch mechanism.
* @return AppProxy instance
*/
function newAppInstance(bytes32 _name, address _appBase, bool _setDefault)
function newAppInstance(bytes32 _appId, address _appBase, bool _setDefault)
public
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _name))
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _appId))
returns (ERCProxy appProxy)
{
_setAppIfNew(APP_BASES_NAMESPACE, _name, _appBase);
appProxy = newAppProxy(this, _name);
_setAppIfNew(APP_BASES_NAMESPACE, _appId, _appBase);
appProxy = newAppProxy(this, _appId);
// By calling setApp directly and not the internal functions, we make sure the params are checked
// and it will only succeed if sender has permissions to set something to the namespace.
if (_setDefault) {
setApp(APP_ADDR_NAMESPACE, _name, appProxy);
setApp(APP_ADDR_NAMESPACE, _appId, appProxy);
}
}

/**
* @dev Create a new pinned instance of an app linked to this kernel
* @param _name Name of the app
* @param _appId Identifier for app
* @param _appBase Address of the app's base implementation
* @return AppProxy instance
*/
function newPinnedAppInstance(bytes32 _name, address _appBase)
function newPinnedAppInstance(bytes32 _appId, address _appBase)
public
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _name))
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _appId))
returns (ERCProxy appProxy)
{
return newPinnedAppInstance(_name, _appBase, false);
return newPinnedAppInstance(_appId, _appBase, false);
}

/**
* @dev Create a new pinned instance of an app linked to this kernel and set
* its base implementation if it was not already set
* @param _name Name of the app
* @param _appId Identifier for app
* @param _appBase Address of the app's base implementation
* @param _setDefault Whether the app proxy app is the default one.
* Useful when the Kernel needs to know of an instance of a particular app,
* like Vault for escape hatch mechanism.
* @return AppProxy instance
*/
function newPinnedAppInstance(bytes32 _name, address _appBase, bool _setDefault)
function newPinnedAppInstance(bytes32 _appId, address _appBase, bool _setDefault)
public
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _name))
auth(APP_MANAGER_ROLE, arr(APP_BASES_NAMESPACE, _appId))
returns (ERCProxy appProxy)
{
_setAppIfNew(APP_BASES_NAMESPACE, _name, _appBase);
appProxy = newAppProxyPinned(this, _name);
_setAppIfNew(APP_BASES_NAMESPACE, _appId, _appBase);
appProxy = newAppProxyPinned(this, _appId);
// By calling setApp directly and not the internal functions, we make sure the params are checked
// and it will only succeed if sender has permissions to set something to the namespace.
if (_setDefault) {
setApp(APP_ADDR_NAMESPACE, _name, appProxy);
setApp(APP_ADDR_NAMESPACE, _appId, appProxy);
}
}

/**
* @dev Set the resolving address of an app instance or base implementation
* @param _namespace App namespace to use
* @param _name Name of the app
* @param _app Address of the app
* @param _appId Identifier for app
* @param _app Address of the app instance or base implementation
* @return ID of app
*/
function setApp(bytes32 _namespace, bytes32 _name, address _app)
function setApp(bytes32 _namespace, bytes32 _appId, address _app)
public
auth(APP_MANAGER_ROLE, arr(_namespace, _name))
returns (bytes32 id)
auth(APP_MANAGER_ROLE, arr(_namespace, _appId))
{
return _setApp(_namespace, _name, _app);
_setApp(_namespace, _appId, _app);
}

/**
* @dev Set the default vault id for the escape hatch mechanism
* @param _name Name of the app
* @param _recoveryVaultAppId Identifier of the recovery vault app
*/
function setRecoveryVaultId(bytes32 _name) public auth(APP_MANAGER_ROLE, arr(APP_ADDR_NAMESPACE, _name)) {
recoveryVaultId = keccak256(abi.encodePacked(APP_ADDR_NAMESPACE, _name));
function setRecoveryVaultAppId(bytes32 _recoveryVaultAppId)
public
auth(APP_MANAGER_ROLE, arr(APP_ADDR_NAMESPACE, _recoveryVaultAppId))
{
recoveryVaultAppId = _recoveryVaultAppId;
}

/**
* @dev Get the address of an app instance or base implementation
* @param _id App identifier
* @param _namespace App namespace to use
* @param _appId Identifier for app
* @return Address of the app
*/
function getApp(bytes32 _id) public view returns (address) {
return apps[_id];
function getApp(bytes32 _namespace, bytes32 _appId) public view returns (address) {
return apps[_namespace][_appId];
}

/**
* @dev Get the address of the recovery Vault instance (to recover funds)
* @return Address of the Vault
*/
function getRecoveryVault() public view returns (address) {
return apps[recoveryVaultId];
return apps[APP_ADDR_NAMESPACE][recoveryVaultAppId];
}

/**
* @dev Get the installed ACL app
* @return ACL app
*/
function acl() public view returns (IACL) {
return IACL(getApp(ACL_APP));
return IACL(getApp(APP_ADDR_NAMESPACE, ACL_APP_ID));
}

/**
Expand All @@ -186,24 +190,19 @@ contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecover
defaultAcl.hasPermission(_who, _where, _what, _how);
}

function _setApp(bytes32 _namespace, bytes32 _name, address _app) internal returns (bytes32 id) {
function _setApp(bytes32 _namespace, bytes32 _appId, address _app) internal {
require(isContract(_app));
id = keccak256(abi.encodePacked(_namespace, _name));
apps[id] = _app;
emit SetApp(_namespace, _name, id, _app);
apps[_namespace][_appId] = _app;
emit SetApp(_namespace, _appId, _app);
}

function _setAppIfNew(bytes32 _namespace, bytes32 _name, address _app) internal returns (bytes32 id) {
require(isContract(_app));

id = keccak256(abi.encodePacked(_namespace, _name));

address app = getApp(id);
function _setAppIfNew(bytes32 _namespace, bytes32 _appId, address _app) internal {
address app = getApp(_namespace, _appId);
if (app != address(0)) {
// The only way to set an app is if it passes the isContract check, so no need to check it again
require(app == _app);
} else {
apps[id] = _app;
emit SetApp(_namespace, _name, id, _app);
_setApp(_namespace, _appId, _app);
}
}

Expand Down
19 changes: 19 additions & 0 deletions contracts/kernel/KernelConstants.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
pragma solidity 0.4.24;


contract KernelConstants {
/*
bytes32 constant public CORE_NAMESPACE = keccak256("core");
bytes32 constant public APP_BASES_NAMESPACE = keccak256("base");
bytes32 constant public APP_ADDR_NAMESPACE = keccak256("app");
bytes32 constant public KERNEL_APP_ID = apmNamehash("kernel");
bytes32 constant public ACL_APP_ID = apmNamehash("acl");
*/
bytes32 constant public CORE_NAMESPACE = 0xc681a85306374a5ab27f0bbc385296a54bcd314a1948b6cf61c4ea1bc44bb9f8;
bytes32 constant public APP_BASES_NAMESPACE = 0xf1f3eb40f5bc1ad1344716ced8b8a0431d840b5783aea1fd01786bc26f35ac0f;
bytes32 constant public APP_ADDR_NAMESPACE = 0xd6f028ca0e8edb4a8c9757ca4fdccab25fa1e0317da1188108f7d2dee14902fb;

bytes32 constant public KERNEL_APP_ID = 0x3b4bf6bf3ad5000ecf0f989d5befde585c6860fea3e574a4fab4c49d1c177d9c;
bytes32 constant public ACL_APP_ID = 0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a;
}
4 changes: 2 additions & 2 deletions contracts/kernel/KernelProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ contract KernelProxy is KernelStorage, IsContract, DepositableDelegateProxy {
*/
constructor(IKernel _kernelImpl) public {
require(isContract(address(_kernelImpl)));
apps[KERNEL_APP] = _kernelImpl;
apps[CORE_NAMESPACE][KERNEL_APP_ID] = _kernelImpl;
}

/**
Expand All @@ -28,6 +28,6 @@ contract KernelProxy is KernelStorage, IsContract, DepositableDelegateProxy {
* @dev ERC897, the address the proxy would delegate calls to
*/
function implementation() public view returns (address) {
return apps[KERNEL_APP];
return apps[CORE_NAMESPACE][KERNEL_APP_ID];
}
}
29 changes: 3 additions & 26 deletions contracts/kernel/KernelStorage.sol
Original file line number Diff line number Diff line change
@@ -1,32 +1,9 @@
pragma solidity 0.4.24;


contract KernelConstants {
/*
bytes32 constant public CORE_NAMESPACE = keccak256("core");
bytes32 constant public APP_BASES_NAMESPACE = keccak256("base");
bytes32 constant public APP_ADDR_NAMESPACE = keccak256("app");
bytes32 constant public KERNEL_APP_ID = apmNamehash("kernel");
bytes32 constant public KERNEL_APP = keccak256(CORE_NAMESPACE, KERNEL_APP_ID);
bytes32 constant public ACL_APP_ID = apmNamehash("acl");
bytes32 constant public ACL_APP = keccak256(APP_ADDR_NAMESPACE, ACL_APP_ID);
*/
bytes32 constant public CORE_NAMESPACE = 0xc681a85306374a5ab27f0bbc385296a54bcd314a1948b6cf61c4ea1bc44bb9f8;
bytes32 constant public APP_BASES_NAMESPACE = 0xf1f3eb40f5bc1ad1344716ced8b8a0431d840b5783aea1fd01786bc26f35ac0f;
bytes32 constant public APP_ADDR_NAMESPACE = 0xd6f028ca0e8edb4a8c9757ca4fdccab25fa1e0317da1188108f7d2dee14902fb;

bytes32 constant public KERNEL_APP_ID = 0x3b4bf6bf3ad5000ecf0f989d5befde585c6860fea3e574a4fab4c49d1c177d9c;
bytes32 constant public KERNEL_APP = 0x2b7d19d0575c228f8d9326801e14149d284dc5bb7b1541c5ad712ae4b2fcaadb;

bytes32 constant public ACL_APP_ID = 0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a;
bytes32 constant public ACL_APP = 0x4b8e03a458a6ccec5d9077c2490964c1333dd3c72e2db408d7d9a7a36ef5c41a;

}
import "./KernelConstants.sol";


contract KernelStorage is KernelConstants {
mapping (bytes32 => address) public apps;
bytes32 public recoveryVaultId;
mapping (bytes32 => mapping (bytes32 => address)) public apps;
bytes32 public recoveryVaultAppId;
}
10 changes: 1 addition & 9 deletions contracts/test/mocks/KeccakConstants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,11 @@ contract KeccakConstants is APMNamehash {
bytes32 constant public APP_ADDR_NAMESPACE = keccak256(abi.encodePacked("app"));

bytes32 constant public KERNEL_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("kernel")));
bytes32 constant public KERNEL_APP = keccak256(abi.encodePacked(CORE_NAMESPACE, KERNEL_APP_ID));

bytes32 constant public ACL_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("acl")));
bytes32 constant public ACL_APP = keccak256(abi.encodePacked(APP_ADDR_NAMESPACE, ACL_APP_ID));

bytes32 constant public APP_MANAGER_ROLE = keccak256(abi.encodePacked("APP_MANAGER_ROLE"));

bytes32 constant public DEFAULT_VAULT_ID = keccak256(
abi.encodePacked(
APP_ADDR_NAMESPACE, keccak256(abi.encodePacked(APM_NODE, keccak256("vault")))
)
);
bytes32 constant public DEFAULT_VAULT_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("vault")));

// ENS
bytes32 constant public ENS_ROOT = bytes32(0);
Expand All @@ -46,7 +39,6 @@ contract KeccakConstants is APMNamehash {

// EVMScriptRegistry
bytes32 constant public EVMSCRIPT_REGISTRY_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("evmreg")));
bytes32 constant public EVMSCRIPT_REGISTRY_APP = keccak256(abi.encodePacked(APP_ADDR_NAMESPACE, EVMSCRIPT_REGISTRY_APP_ID));
bytes32 constant public REGISTRY_ADD_EXECUTOR_ROLE = keccak256("REGISTRY_ADD_EXECUTOR_ROLE");
bytes32 constant public REGISTRY_MANAGER_ROLE = keccak256(abi.encodePacked("REGISTRY_MANAGER_ROLE"));

Expand Down
Loading

0 comments on commit 5b880fc

Please sign in to comment.