Skip to content

Commit

Permalink
Merge b2b7e87 into 47bd8f7
Browse files Browse the repository at this point in the history
  • Loading branch information
@izqui
izqui committed Oct 16, 2018
2 parents 47bd8f7 + b2b7e87 commit 1d88417
Show file tree
Hide file tree
Showing 15 changed files with 107 additions and 49 deletions.
10 changes: 7 additions & 3 deletions contracts/acl/ACL.sol
View file
Expand Up @@ -48,14 +48,18 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers {

uint256 internal constant ORACLE_CHECK_GAS = 30000;

string private constant ERROR_AUTH_INIT_KERNEL = "ACL_AUTH_INIT_KERNEL";
string private constant ERROR_AUTH_NO_MANAGER = "ACL_AUTH_NO_MANAGER";
string private constant ERROR_EXISTENT_MANAGER = "ACL_EXISTENT_MANAGER";

modifier onlyPermissionManager(address _app, bytes32 _role) {
require(msg.sender == getPermissionManager(_app, _role));
require(msg.sender == getPermissionManager(_app, _role), ERROR_AUTH_NO_MANAGER);
_;
}

modifier noPermissionManager(address _app, bytes32 _role) {
// only allow permission creation (or re-creation) when there is no manager
require(getPermissionManager(_app, _role) == address(0));
require(getPermissionManager(_app, _role) == address(0), ERROR_EXISTENT_MANAGER);
_;
}

Expand All @@ -70,7 +74,7 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers {
*/
function initialize(address _permissionsCreator) public onlyInit {
initialized();
require(msg.sender == address(kernel()));
require(msg.sender == address(kernel()), ERROR_AUTH_INIT_KERNEL);

_createPermission(_permissionsCreator, this, CREATE_PERMISSIONS_ROLE, _permissionsCreator);
}
Expand Down
9 changes: 6 additions & 3 deletions contracts/apm/APMRegistry.sol
View file
Expand Up @@ -24,6 +24,9 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
// bytes32 public constant CREATE_REPO_ROLE = keccak256("CREATE_REPO_ROLE");
bytes32 public constant CREATE_REPO_ROLE = 0x2a9494d64846c9fdbf0158785aa330d8bc9caf45af27fa0e8898eb4d55adcea6;

string private constant ERROR_INIT_PERMISSIONS = "APMREG_INIT_PERMISSIONS";
string private constant ERROR_EMPTY_NAME = "APMREG_EMPTY_NAME";

event NewRepo(bytes32 id, string name, address repo);

/**
Expand All @@ -40,8 +43,8 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {

// Check APM has all permissions it needss
ACL acl = ACL(kernel().acl());
require(acl.hasPermission(this, registrar, registrar.CREATE_NAME_ROLE()));
require(acl.hasPermission(this, acl, acl.CREATE_PERMISSIONS_ROLE()));
require(acl.hasPermission(this, registrar, registrar.CREATE_NAME_ROLE()), ERROR_INIT_PERMISSIONS);
require(acl.hasPermission(this, acl, acl.CREATE_PERMISSIONS_ROLE()), ERROR_INIT_PERMISSIONS);
}

/**
Expand Down Expand Up @@ -81,7 +84,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
}

function _newRepo(string _name, address _dev) internal returns (Repo) {
require(bytes(_name).length > 0);
require(bytes(_name).length > 0, ERROR_EMPTY_NAME);

Repo repo = newClonedRepo();

Expand Down
13 changes: 10 additions & 3 deletions contracts/apm/Repo.sol
View file
Expand Up @@ -20,6 +20,10 @@ contract Repo is AragonApp {
// bytes32 public constant CREATE_VERSION_ROLE = keccak256("CREATE_VERSION_ROLE");
bytes32 public constant CREATE_VERSION_ROLE = 0x1f56cfecd3595a2e6cc1a7e6cb0b20df84cdbd92eff2fee554e70e4e45a9a7d8;

string private constant ERROR_INVALID_BUMP = "REPO_INVALID_BUMP";
string private constant ERROR_INVALID_VERSION = "REPO_INVALID_VERSION";
string private constant ERROR_INEXISTENT_VERSION = "REPO_INEXISTENT_VERSION";

event NewVersion(uint256 versionId, uint16[3] semanticVersion);

/**
Expand Down Expand Up @@ -56,10 +60,13 @@ contract Repo is AragonApp {
contractAddress = lastVersion.contractAddress;
}
// Only allows smart contract change on major version bumps
require(lastVersion.contractAddress == contractAddress || _newSemanticVersion[0] > lastVersion.semanticVersion[0]);
require(
lastVersion.contractAddress == contractAddress || _newSemanticVersion[0] > lastVersion.semanticVersion[0],
ERROR_INVALID_VERSION
);
}

require(isValidBump(lastSematicVersion, _newSemanticVersion));
require(isValidBump(lastSematicVersion, _newSemanticVersion), ERROR_INVALID_BUMP);

uint256 versionId = versionsNextIndex++;
versions[versionId] = Version(_newSemanticVersion, contractAddress, _contentURI);
Expand Down Expand Up @@ -90,7 +97,7 @@ contract Repo is AragonApp {
}

function getByVersionId(uint _versionId) public view returns (uint16[3] semanticVersion, address contractAddress, bytes contentURI) {
require(_versionId > 0 && _versionId < versionsNextIndex);
require(_versionId > 0 && _versionId < versionsNextIndex, ERROR_INEXISTENT_VERSION);
Version storage version = versions[_versionId];
return (version.semanticVersion, version.contractAddress, version.contentURI);
}
Expand Down
6 changes: 4 additions & 2 deletions contracts/apps/AragonApp.sol
View file
Expand Up @@ -17,13 +17,15 @@ import "../acl/ACLSyntaxSugar.sol";
// ACLSyntaxSugar and EVMScriptRunner are not directly used by this contract, but are included so
// that they are automatically usable by subclassing contracts
contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, EVMScriptRunner, ACLSyntaxSugar {
string private constant ERROR_AUTH_FAILED = "APP_AUTH_FAILED";

modifier auth(bytes32 _role) {
require(canPerform(msg.sender, _role, new uint256[](0)));
require(canPerform(msg.sender, _role, new uint256[](0)), ERROR_AUTH_FAILED);
_;
}

modifier authP(bytes32 _role, uint256[] _params) {
require(canPerform(msg.sender, _role, _params));
require(canPerform(msg.sender, _role, _params), ERROR_AUTH_FAILED);
_;
}

Expand Down
7 changes: 5 additions & 2 deletions contracts/common/Initializable.sol
View file
Expand Up @@ -14,13 +14,16 @@ contract Initializable is TimeHelpers {
// keccak256("aragonOS.initializable.initializationBlock")
bytes32 internal constant INITIALIZATION_BLOCK_POSITION = 0xebb05b386a8d34882b8711d156f463690983dc47815980fb82aeeff1aa43579e;

string private constant ERROR_ALREADY_INITIALIZED = "INIT_ALREADY_INITIALIZED";
string private constant ERROR_NOT_INITIALIZED = "INIT_NOT_INITIALIZED";

modifier onlyInit {
require(getInitializationBlock() == 0);
require(getInitializationBlock() == 0, ERROR_ALREADY_INITIALIZED);
_;
}

modifier isInitialized {
require(hasInitialized());
require(hasInitialized(), ERROR_NOT_INITIALIZED);
_;
}

Expand Down
4 changes: 3 additions & 1 deletion contracts/common/Uint256Helpers.sol
View file
Expand Up @@ -4,8 +4,10 @@ pragma solidity ^0.4.24;
library Uint256Helpers {
uint256 public constant MAX_UINT64 = uint64(-1);

string private constant ERROR_NUMBER_TOO_BIG = "UINT64_NUMBER_TOO_BIG";

function toUint64(uint256 a) internal pure returns (uint64) {
require(a <= MAX_UINT64);
require(a <= MAX_UINT64, ERROR_NUMBER_TOO_BIG);
return uint64(a);
}
}
7 changes: 5 additions & 2 deletions contracts/common/VaultRecoverable.sol
View file
Expand Up @@ -11,15 +11,18 @@ import "./IVaultRecoverable.sol";


contract VaultRecoverable is IVaultRecoverable, EtherTokenConstant, IsContract {
string private constant ERROR_DISALLOWED = "RECOVER_DISALLOWED";
string private constant ERROR_VAULT_NOT_CONTRACT = "RECOVER_VAULT_NOT_CONTRACT";

/**
* @notice Send funds to recovery Vault. This contract should never receive funds,
* but in case it does, this function allows one to recover them.
* @param _token Token balance to be sent to recovery vault.
*/
function transferToVault(address _token) external {
require(allowRecoverability(_token));
require(allowRecoverability(_token), ERROR_DISALLOWED);
address vault = getRecoveryVault();
require(isContract(vault));
require(isContract(vault), ERROR_VAULT_NOT_CONTRACT);

if (_token == ETH) {
vault.transfer(address(this).balance);
Expand Down
10 changes: 7 additions & 3 deletions contracts/ens/ENSSubdomainRegistrar.sol
View file
Expand Up @@ -17,6 +17,10 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {
bytes32 public constant DELETE_NAME_ROLE = 0x03d74c8724218ad4a99859bcb2d846d39999449fd18013dd8d69096627e68622;
bytes32 public constant POINT_ROOTNODE_ROLE = 0x9ecd0e7bddb2e241c41b595a436c4ea4fd33c9fa0caa8056acf084fc3aa3bfbe;

string private constant ERROR_NO_NODE_OWNERSHIP = "ENSSUB_NO_NODE_OWNERSHIP";
string private constant ERROR_NAME_EXISTS = "ENSSUB_NAME_EXISTS";
string private constant ERROR_NAME_DOESNT_EXIST = "ENSSUB_DOESNT_EXIST";

AbstractENS public ens;
bytes32 public rootNode;

Expand All @@ -27,7 +31,7 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {
initialized();

// We need ownership to create subnodes
require(_ens.owner(_rootNode) == address(this));
require(_ens.owner(_rootNode) == address(this), ERROR_NO_NODE_OWNERSHIP);

ens = _ens;
rootNode = _rootNode;
Expand All @@ -47,7 +51,7 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {

address currentOwner = ens.owner(node);

require(currentOwner != address(0)); // fail if deleting unset name
require(currentOwner != address(0), ERROR_NAME_DOESNT_EXIST); // fail if deleting unset name

if (currentOwner != address(this)) { // needs to reclaim ownership so it can set resolver
ens.setSubnodeOwner(rootNode, _label, this);
Expand All @@ -65,7 +69,7 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {

function _createName(bytes32 _label, address _owner) internal returns (bytes32 node) {
node = getNodeForLabel(_label);
require(ens.owner(node) == address(0)); // avoid name reset
require(ens.owner(node) == address(0), ERROR_NAME_EXISTS); // avoid name reset

ens.setSubnodeOwner(rootNode, _label, _owner);

Expand Down
10 changes: 7 additions & 3 deletions contracts/evmscript/EVMScriptRegistry.sol
View file
Expand Up @@ -17,6 +17,10 @@ contract EVMScriptRegistry is IEVMScriptRegistry, EVMScriptRegistryConstants, Ar
// bytes32 public constant REGISTRY_MANAGER_ROLE = keccak256("REGISTRY_MANAGER_ROLE");
bytes32 public constant REGISTRY_MANAGER_ROLE = 0xf7a450ef335e1892cb42c8ca72e7242359d7711924b75db5717410da3f614aa3;

string private constant ERROR_INEXISTENT_EXECUTOR = "EVMREG_INEXISTENT_EXECUTOR";
string private constant ERROR_EXECUTOR_ENABLED = "EVMREG_EXECUTOR_ENABLED";
string private constant ERROR_EXECUTOR_DISABLED = "EVMREG_EXECUTOR_DISABLED";

struct ExecutorEntry {
IEVMScriptExecutor executor;
bool enabled;
Expand All @@ -29,7 +33,7 @@ contract EVMScriptRegistry is IEVMScriptRegistry, EVMScriptRegistryConstants, Ar
event DisableExecutor(uint256 indexed executorId, address indexed executorAddress);

modifier executorExists(uint256 _executorId) {
require(_executorId > 0 && _executorId < executorsNextIndex);
require(_executorId > 0 && _executorId < executorsNextIndex, ERROR_INEXISTENT_EXECUTOR);
_;
}

Expand Down Expand Up @@ -65,7 +69,7 @@ contract EVMScriptRegistry is IEVMScriptRegistry, EVMScriptRegistryConstants, Ar
// Note that we don't need to check for an executor's existence in this case, as only
// existing executors can be enabled
ExecutorEntry storage executorEntry = executors[_executorId];
require(executorEntry.enabled);
require(executorEntry.enabled, ERROR_EXECUTOR_DISABLED);
executorEntry.enabled = false;
emit DisableExecutor(_executorId, executorEntry.executor);
}
Expand All @@ -80,7 +84,7 @@ contract EVMScriptRegistry is IEVMScriptRegistry, EVMScriptRegistryConstants, Ar
executorExists(_executorId)
{
ExecutorEntry storage executorEntry = executors[_executorId];
require(!executorEntry.enabled);
require(!executorEntry.enabled, ERROR_EXECUTOR_ENABLED);
executorEntry.enabled = true;
emit EnableExecutor(_executorId, executorEntry.executor);
}
Expand Down
14 changes: 9 additions & 5 deletions contracts/evmscript/EVMScriptRunner.sol
View file
Expand Up @@ -13,6 +13,10 @@ import "../common/Initializable.sol";


contract EVMScriptRunner is AppStorage, Initializable, EVMScriptRegistryConstants, KernelConstants {
string private constant ERROR_EXECUTOR_UNAVAILABLE = "EVMRUN_EXECUTOR_UNAVAILABLE";
string private constant ERROR_EXECUTION_REVERTED = "EVMRUN_EXECUTION_REVERTED";
string private constant ERROR_PROTECTED_STATE_MODIFIED = "EVMRUN_PROTECTED_STATE_MODIFIED";

event ScriptResult(address indexed executor, bytes script, bytes input, bytes returnData);

function getExecutor(bytes _script) public view returns (IEVMScriptExecutor) {
Expand All @@ -27,11 +31,11 @@ contract EVMScriptRunner is AppStorage, Initializable, EVMScriptRegistryConstant
{
// TODO: Too much data flying around, maybe extracting spec id here is cheaper
IEVMScriptExecutor executor = getExecutor(_script);
require(address(executor) != address(0));
require(address(executor) != address(0), ERROR_EXECUTOR_UNAVAILABLE);

bytes4 sig = executor.execScript.selector;

require(address(executor).delegatecall(abi.encodeWithSelector(sig, _script, _input, _blacklist)));
bytes memory calldata = abi.encodeWithSelector(sig, _script, _input, _blacklist);
require(address(executor).delegatecall(calldata), ERROR_EXECUTION_REVERTED);

bytes memory output = returnedDataDecoded();

Expand Down Expand Up @@ -66,7 +70,7 @@ contract EVMScriptRunner is AppStorage, Initializable, EVMScriptRegistryConstant
address preKernel = address(kernel());
bytes32 preAppId = appId();
_; // exec
require(address(kernel()) == preKernel);
require(appId() == preAppId);
require(address(kernel()) == preKernel, ERROR_PROTECTED_STATE_MODIFIED);
require(appId() == preAppId, ERROR_PROTECTED_STATE_MODIFIED);
}
}
14 changes: 10 additions & 4 deletions contracts/evmscript/executors/CallsScript.sol
View file
Expand Up @@ -12,6 +12,10 @@ contract CallsScript is BaseEVMScriptExecutor {
// bytes32 internal constant EXECUTOR_TYPE = keccak256("CALLS_SCRIPT");
bytes32 internal constant EXECUTOR_TYPE = 0x2dc858a00f3e417be1394b87c07158e989ec681ce8cc68a9093680ac1a870302;

string private constant ERROR_BLACKLISTED_CALL = "EVMCALLS_BLACKLISTED_CALL";
string private constant ERROR_INVALID_LENGTH = "EVMCALLS_INVALID_LENGTH";
string private constant ERROR_CALL_REVERTED = "EVMCALLS_CALL_REVERTED";

event LogScriptCall(address indexed sender, address indexed src, address indexed dst);

/**
Expand All @@ -27,7 +31,7 @@ contract CallsScript is BaseEVMScriptExecutor {
address contractAddress = _script.addressAt(location);
// Check address being called is not blacklist
for (uint i = 0; i < _blacklist.length; i++) {
require(contractAddress != _blacklist[i]);
require(contractAddress != _blacklist[i], ERROR_BLACKLISTED_CALL);
}

// logged before execution to ensure event ordering in receipt
Expand All @@ -40,12 +44,14 @@ contract CallsScript is BaseEVMScriptExecutor {

// compute end of script / next location
location = startOffset + calldataLength;
require(location <= _script.length);
require(location <= _script.length, ERROR_INVALID_LENGTH);

bool success;
assembly {
let success := call(sub(gas, 5000), contractAddress, 0, calldataStart, calldataLength, 0, 0)
switch success case 0 { revert(0, 0) }
success := call(sub(gas, 5000), contractAddress, 0, calldataStart, calldataLength, 0, 0)
}

require(success, ERROR_CALL_REVERTED);
}
}

Expand Down
10 changes: 7 additions & 3 deletions contracts/kernel/Kernel.sol
View file
Expand Up @@ -18,6 +18,10 @@ contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecover
bytes32 public constant APP_MANAGER_ROLE = 0xb6d92708f3d4817afc106147d969e229ced5c46e65e0a5002a0d391287762bd0;
bytes32 public constant DEFAULT_VAULT_APP_ID = 0x7e852e0fcfce6551c13800f1e7476f982525c2b5277ba14b24339c68416336d1;

string private constant ERROR_APP_NOT_CONTRACT = "KERNEL_APP_NOT_CONTRACT";
string private constant ERROR_INVALID_APP_CHANGE = "KERNEL_INVALID_APP_CHANGE";
string private constant ERROR_AUTH_FAILED = "KERNEL_AUTH_FAILED";

/**
* @dev Constructor that allows the deployer to choose if the base instance should be petrified immediately.
* @param _shouldPetrify Immediately petrify this instance so that it can never be initialized
Expand Down Expand Up @@ -198,7 +202,7 @@ contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecover
}

function _setApp(bytes32 _namespace, bytes32 _appId, address _app) internal {
require(isContract(_app));
require(isContract(_app), ERROR_APP_NOT_CONTRACT);
apps[_namespace][_appId] = _app;
emit SetApp(_namespace, _appId, _app);
}
Expand All @@ -207,7 +211,7 @@ contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecover
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);
require(app == _app, ERROR_INVALID_APP_CHANGE);
} else {
_setApp(_namespace, _appId, _app);
}
Expand All @@ -221,7 +225,7 @@ contract Kernel is IKernel, KernelStorage, Petrifiable, IsContract, VaultRecover
mstore(how, byteLength)
}
// Params is invalid from this point fwd
require(hasPermission(msg.sender, address(this), _role, how));
require(hasPermission(msg.sender, address(this), _role, how), ERROR_AUTH_FAILED);
_;
}
}

0 comments on commit 1d88417

Please sign in to comment.