Skip to content

Commit

Permalink
Agent app: EVM (aragon#580)
Browse files Browse the repository at this point in the history
* Actor app: inherit Vault and test vault functionality

* Actor app: implement execute action

* Actor app: implement permissioned forwarding

* Actor app: improve tests

* Actor app: signatures

* CI: travis wait on npm install

* Actor app: lint and enable CI

* Add max gas for static calls

* chore: upgrade to [email protected]

* Address review comments

* Lint

* test: add comments, cosmetic changes, and more tests

* SignatureValidator: fix compile warnings

* test: add context headers

* Metadata: add roles to arapp, remove webapp artifacts

* Radspec strings

* Travis: allow actor coverage to fail, tracking fix in aragon#658

* Prevent infinite loop by prohibiting setting itself as the designated signer

* Radspec

* Actor -> Agent rename

* Add status to arapp and rename constants

* Update ERC1271 implementation

* Add ERC1271 interfaceId

* Rename constants

* Rename calldata

* Refactor SignatureValidator.sol

* Refactor signature validation tests and test EIP712 signatures

* Handle empty signatures and undefined signature modes

* Refactor SignatureValidator: handle ERC1271 checks in library

* Test ERC1271 signature wrapping

* cosmetic: fix whitespace

* Agent: always allow execute to transfer ETH (aragon#651)

* cosmetic: fix whitespace and add EIPs hash

* chore: upgrade aragonOS and use test-helper's contracts

* cosmetic: fix compile errors

* cosmetic: fix linter

* chore: fix travis allowed to fail matrix

* test: ignore all test contracts from coverage

* Actor: reuse Vault tests (aragon#668)

The Vault now exports its tests via a sharable utility.

See f41519a for the changes made to the original tests to make them sharable.

* Move agent from future-apps/ to apps/

* Refactor popFirstByte to copy to a new array

* Address last review comments

* Lint

* Address very last review comments
  • Loading branch information
izqui authored Feb 15, 2019
1 parent 2c08b81 commit 3de52d2
Show file tree
Hide file tree
Showing 32 changed files with 2,255 additions and 2,484 deletions.
7 changes: 7 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ cache:
- apps/vault/node_modules
- apps/voting/node_modules
- apps/voting/app/node_modules
- future-apps/agent/node_modules
- future-apps/payroll/node_modules
- shared/migrations/node_modules
- shared/minime/node_modules
- shared/test-helpers/node_modules
Expand All @@ -24,12 +26,14 @@ env:
- INSTALL_FRONTEND=false
matrix:
- TASK=lint
- TASK=test:agent
- TASK=test:finance
- TASK=test:survey
- TASK=test:token-manager
- TASK=test:token-manager:app INSTALL_FRONTEND=true
- TASK=test:vault
- TASK=test:voting
- TASK=coverage:agent
- TASK=coverage:finance
- TASK=coverage:survey
- TASK=coverage:token-manager
Expand All @@ -38,7 +42,10 @@ env:
- TASK=test:shared
matrix:
allow_failures:
- env: TASK=coverage:agent
- env: TASK=coverage:finance
install:
- travis_wait 60 npm install
before_script:
- npm prune
script:
Expand Down
7 changes: 7 additions & 0 deletions apps/agent/.solcover.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
norpc: true,
copyPackages: ['@aragon/os', '@aragon/apps-vault'],
skipFiles: [
'test'
]
}
1 change: 1 addition & 0 deletions apps/agent/.soliumignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
contracts/test
23 changes: 23 additions & 0 deletions apps/agent/.soliumrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"extends": "solium:all",
"rules": {
"imports-on-top": ["error"],
"variable-declarations": ["error"],
"array-declarations": ["error"],
"operator-whitespace": ["error"],
"lbrace": ["error"],
"mixedcase": 0,
"camelcase": ["error"],
"uppercase": 0,
"no-empty-blocks": ["error"],
"no-unused-vars": ["error"],
"quotes": ["error"],
"indentation": 0,
"whitespace": ["error"],
"deprecated-suicide": ["error"],
"arg-overflow": ["error", 8],
"pragma-on-top": ["error"],
"security/enforce-explicit-visibility": ["error"],
"error-reason": 0
}
}
63 changes: 63 additions & 0 deletions apps/agent/arapp.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
{
"status": "experimental",
"environments": {
"default": {
"registry": "0x5f6f7e8cc7346a11ca2def8f827b7a0b612c56a1",
"appName": "agent.aragonpm.eth",
"network": "rpc"
},
"rinkeby": {
"registry": "0x98Df287B6C145399Aaa709692c8D308357bC085D",
"appName": "agent.aragonpm.eth",
"network": "rinkeby"
},
"staging": {
"registry": "0xfe03625ea880a8cba336f9b5ad6e15b0a3b5a939",
"appName": "agent.aragonpm.eth",
"network": "rinkeby"
},
"mainnet": {
"registry": "0x314159265dd8dbb310642f98f50c066173c1259b",
"appName": "agent.aragonpm.eth",
"network": "mainnet"
},
"rinkeby-old": {
"registry": "0xfbae32d1cde62858bc45f51efc8cc4fa1415447e",
"appName": "agent.aragonpm.eth",
"network": "rinkeby"
}
},
"roles": [
{
"name": "Execute actions",
"id": "EXECUTE_ROLE",
"params": [
"Target address",
"ETH value",
"Signature"
]
},
{
"name": "Designate signer",
"id": "DESIGNATE_SIGNER_ROLE",
"params": [
"Designated signer address"
]
},
{
"name": "Presign hash",
"id": "ADD_PRESIGNED_HASH_ROLE",
"params": [
"Hash"
]
},
{
"name": "Run EVM Script",
"id": "RUN_SCRIPT_ROLE",
"params": [
"EVM Script"
]
}
],
"path": "contracts/Agent.sol"
}
1 change: 1 addition & 0 deletions apps/agent/contracts/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/test
147 changes: 147 additions & 0 deletions apps/agent/contracts/Agent.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/*
* SPDX-License-Identitifer: GPL-3.0-or-later
*/

pragma solidity 0.4.24;

import "./SignatureValidator.sol";
import "./standards/IERC165.sol";
import "./standards/ERC1271.sol";

import "@aragon/apps-vault/contracts/Vault.sol";

import "@aragon/os/contracts/common/IForwarder.sol";


contract Agent is IERC165, ERC1271Bytes, IForwarder, IsContract, Vault {
bytes32 public constant EXECUTE_ROLE = keccak256("EXECUTE_ROLE");
bytes32 public constant RUN_SCRIPT_ROLE = keccak256("RUN_SCRIPT_ROLE");
bytes32 public constant ADD_PRESIGNED_HASH_ROLE = keccak256("ADD_PRESIGNED_HASH_ROLE");
bytes32 public constant DESIGNATE_SIGNER_ROLE = keccak256("DESIGNATE_SIGNER_ROLE");

bytes4 private constant ERC165_INTERFACE_ID = 0x01ffc9a7;

string private constant ERROR_EXECUTE_ETH_NO_DATA = "AGENT_EXEC_ETH_NO_DATA";
string private constant ERROR_EXECUTE_TARGET_NOT_CONTRACT = "AGENT_EXEC_TARGET_NO_CONTRACT";
string private constant ERROR_DESIGNATED_TO_SELF = "AGENT_DESIGNATED_TO_SELF";

mapping (bytes32 => bool) public isPresigned;
address public designatedSigner;

event Execute(address indexed sender, address indexed target, uint256 ethValue, bytes data);
event PresignHash(address indexed sender, bytes32 indexed hash);
event SetDesignatedSigner(address indexed sender, address indexed oldSigner, address indexed newSigner);

/**
* @notice Execute '`@radspec(_target, _data)`' on `_target``_ethValue == 0 ? '' : ' (Sending' + @tokenAmount(_ethValue, 0x00) + ')'`
* @param _target Address where the action is being executed
* @param _ethValue Amount of ETH from the contract that is sent with the action
* @param _data Calldata for the action
* @return Exits call frame forwarding the return data of the executed call (either error or success data)
*/
function execute(address _target, uint256 _ethValue, bytes _data)
external // This function MUST always be external as the function performs a low level return, exiting the Agent app execution context
authP(EXECUTE_ROLE, arr(_target, _ethValue, uint256(getSig(_data)))) // TODO: Test that sig bytes are the least significant bytes
{
bool result = _target.call.value(_ethValue)(_data);

if (result) {
emit Execute(msg.sender, _target, _ethValue, _data);
}

assembly {
let size := returndatasize
let ptr := mload(0x40)
returndatacopy(ptr, 0, size)

// revert instead of invalid() bc if the underlying call failed with invalid() it already wasted gas.
// if the call returned error data, forward it
switch result case 0 { revert(ptr, size) }
default { return(ptr, size) }
}
}

/**
* @notice Set `_designatedSigner` as the designated signer of the app, which will be able to sign messages on behalf of the app
* @param _designatedSigner Address that will be able to sign messages on behalf of the app
*/
function setDesignatedSigner(address _designatedSigner)
external
authP(DESIGNATE_SIGNER_ROLE, arr(_designatedSigner))
{
// Prevent an infinite loop by setting the app itself as its designated signer.
// An undetectable loop can be created by setting a different contract as the
// designated signer which calls back into `isValidSignature`.
// Given that `isValidSignature` is always called with just 50k gas, the max
// damage of the loop is wasting 50k gas.
require(_designatedSigner != address(this), ERROR_DESIGNATED_TO_SELF);

address oldDesignatedSigner = designatedSigner;
designatedSigner = _designatedSigner;

emit SetDesignatedSigner(msg.sender, oldDesignatedSigner, _designatedSigner);
}

/**
* @notice Pre-sign hash `_hash`
* @param _hash Hash that will be considered signed regardless of the signature checked with 'isValidSignature()'
*/
function presignHash(bytes32 _hash)
external
authP(ADD_PRESIGNED_HASH_ROLE, arr(_hash))
{
isPresigned[_hash] = true;

emit PresignHash(msg.sender, _hash);
}

function isForwarder() external pure returns (bool) {
return true;
}

function supportsInterface(bytes4 interfaceId) external pure returns (bool) {
return
interfaceId == ERC1271_INTERFACE_ID ||
interfaceId == ERC165_INTERFACE_ID;
}

/**
* @notice Execute the script as the Agent app
* @dev IForwarder interface conformance. Forwards any token holder action.
* @param _evmScript Script being executed
*/
function forward(bytes _evmScript)
public
authP(RUN_SCRIPT_ROLE, arr(getScriptACLParam(_evmScript)))
{
bytes memory input = ""; // no input
address[] memory blacklist = new address[](0); // no addr blacklist, can interact with anything
runScript(_evmScript, input, blacklist);
// We don't need to emit an event here as EVMScriptRunner will emit ScriptResult if successful
}

function isValidSignature(bytes32 hash, bytes signature) public view returns (bytes4) {
// Short-circuit in case the hash was presigned. Optimization as performing calls
// and ecrecover is more expensive than an SLOAD.
if (isPresigned[hash]) {
return returnIsValidSignatureMagicNumber(true);
}

bool isValid = SignatureValidator.isValidSignature(hash, designatedSigner, signature);
return returnIsValidSignatureMagicNumber(isValid);
}

function canForward(address sender, bytes evmScript) public view returns (bool) {
uint256[] memory params = new uint256[](1);
params[0] = getScriptACLParam(evmScript);
return canPerform(sender, RUN_SCRIPT_ROLE, params);
}

function getScriptACLParam(bytes evmScript) internal pure returns (uint256) {
return uint256(keccak256(abi.encodePacked(evmScript)));
}

function getSig(bytes data) internal pure returns (bytes4 sig) {
assembly { sig := add(data, 0x20) }
}
}
Loading

0 comments on commit 3de52d2

Please sign in to comment.