diff --git a/apps/agent/arapp.json b/apps/agent/arapp.json index 7c00d402c7..aa6734555b 100644 --- a/apps/agent/arapp.json +++ b/apps/agent/arapp.json @@ -6,6 +6,11 @@ "appName": "agent.aragonpm.eth", "network": "rpc" }, + "mainnet": { + "registry": "0x314159265dd8dbb310642f98f50c066173c1259b", + "appName": "agent.aragonpm.eth", + "network": "mainnet" + }, "rinkeby": { "registry": "0x98Df287B6C145399Aaa709692c8D308357bC085D", "appName": "agent.aragonpm.eth", @@ -16,23 +21,22 @@ "appName": "agent.aragonpm.eth", "network": "rinkeby" }, - "mainnet": { - "registry": "0x314159265dd8dbb310642f98f50c066173c1259b", - "appName": "agent.aragonpm.eth", - "network": "mainnet" - }, "ropsten": { "registry": "0x6afe2cacee211ea9179992f89dc61ff25c61e923", "appName": "agent.aragonpm.eth", "network": "ropsten" - }, - "rinkeby-old": { - "registry": "0xfbae32d1cde62858bc45f51efc8cc4fa1415447e", - "appName": "agent.aragonpm.eth", - "network": "rinkeby" } }, "roles": [ + { + "name": "Transfer Agent's tokens", + "id": "TRANSFER_ROLE", + "params": [ + "Token address", + "Receiver address", + "Token amount" + ] + }, { "name": "Execute actions", "id": "EXECUTE_ROLE", @@ -43,19 +47,41 @@ ] }, { - "name": "Designate signer", - "id": "DESIGNATE_SIGNER_ROLE", + "name": "Execute safe actions", + "id": "SAFE_EXECUTE_ROLE", "params": [ - "Designated signer address" + "Target address", + "Signature" ] }, { - "name": "Presign hash", + "name": "Add protected tokens", + "id": "ADD_PROTECTED_TOKEN_ROLE", + "params": [ + "Token" + ] + }, + { + "name": "Remove protected tokens", + "id": "REMOVE_PROTECTED_TOKEN_ROLE", + "params": [ + "Token" + ] + }, + { + "name": "Add presigned hashes", "id": "ADD_PRESIGNED_HASH_ROLE", "params": [ "Hash" ] }, + { + "name": "Set designated signer", + "id": "DESIGNATE_SIGNER_ROLE", + "params": [ + "Designated signer address" + ] + }, { "name": "Run EVM Script", "id": "RUN_SCRIPT_ROLE", @@ -65,4 +91,4 @@ } ], "path": "contracts/Agent.sol" -} \ No newline at end of file +} diff --git a/apps/agent/contracts/Agent.sol b/apps/agent/contracts/Agent.sol index 6290824058..cd10e00f5c 100644 --- a/apps/agent/contracts/Agent.sol +++ b/apps/agent/contracts/Agent.sol @@ -14,19 +14,46 @@ import "@aragon/os/contracts/common/IForwarder.sol"; contract Agent is IERC165, ERC1271Bytes, IForwarder, IsContract, Vault { + /* Hardcoded constants to save gas bytes32 public constant EXECUTE_ROLE = keccak256("EXECUTE_ROLE"); - bytes32 public constant RUN_SCRIPT_ROLE = keccak256("RUN_SCRIPT_ROLE"); + bytes32 public constant SAFE_EXECUTE_ROLE = keccak256("SAFE_EXECUTE_ROLE"); + bytes32 public constant ADD_PROTECTED_TOKEN_ROLE = keccak256("ADD_PROTECTED_TOKEN_ROLE"); + bytes32 public constant REMOVE_PROTECTED_TOKEN_ROLE = keccak256("REMOVE_PROTECTED_TOKEN_ROLE"); bytes32 public constant ADD_PRESIGNED_HASH_ROLE = keccak256("ADD_PRESIGNED_HASH_ROLE"); bytes32 public constant DESIGNATE_SIGNER_ROLE = keccak256("DESIGNATE_SIGNER_ROLE"); + bytes32 public constant RUN_SCRIPT_ROLE = keccak256("RUN_SCRIPT_ROLE"); + */ + + bytes32 public constant EXECUTE_ROLE = 0xcebf517aa4440d1d125e0355aae64401211d0848a23c02cc5d29a14822580ba4; + bytes32 public constant SAFE_EXECUTE_ROLE = 0x0a1ad7b87f5846153c6d5a1f761d71c7d0cfd122384f56066cd33239b7933694; + bytes32 public constant ADD_PROTECTED_TOKEN_ROLE = 0x6eb2a499556bfa2872f5aa15812b956cc4a71b4d64eb3553f7073c7e41415aaa; + bytes32 public constant REMOVE_PROTECTED_TOKEN_ROLE = 0x71eee93d500f6f065e38b27d242a756466a00a52a1dbcd6b4260f01a8640402a; + bytes32 public constant ADD_PRESIGNED_HASH_ROLE = 0x0b29780bb523a130b3b01f231ef49ed2fa2781645591a0b0a44ca98f15a5994c; + bytes32 public constant DESIGNATE_SIGNER_ROLE = 0x23ce341656c3f14df6692eebd4757791e33662b7dcf9970c8308303da5472b7c; + bytes32 public constant RUN_SCRIPT_ROLE = 0xb421f7ad7646747f3051c50c0b8e2377839296cd4973e27f63821d73e390338f; + + uint256 public constant PROTECTED_TOKENS_CAP = 10; bytes4 private constant ERC165_INTERFACE_ID = 0x01ffc9a7; + string private constant ERROR_TARGET_PROTECTED = "AGENT_TARGET_PROTECTED"; + string private constant ERROR_PROTECTED_TOKENS_MODIFIED = "AGENT_PROTECTED_TOKENS_MODIFIED"; + string private constant ERROR_PROTECTED_BALANCE_LOWERED = "AGENT_PROTECTED_BALANCE_LOWERED"; + string private constant ERROR_TOKENS_CAP_REACHED = "AGENT_TOKENS_CAP_REACHED"; + string private constant ERROR_TOKEN_NOT_ERC20 = "AGENT_TOKEN_NOT_ERC20"; + string private constant ERROR_TOKEN_ALREADY_PROTECTED = "AGENT_TOKEN_ALREADY_PROTECTED"; + string private constant ERROR_TOKEN_NOT_PROTECTED = "AGENT_TOKEN_NOT_PROTECTED"; string private constant ERROR_DESIGNATED_TO_SELF = "AGENT_DESIGNATED_TO_SELF"; + string private constant ERROR_CAN_NOT_FORWARD = "AGENT_CAN_NOT_FORWARD"; mapping (bytes32 => bool) public isPresigned; address public designatedSigner; + address[] public protectedTokens; + event SafeExecute(address indexed sender, address indexed target, bytes data); event Execute(address indexed sender, address indexed target, uint256 ethValue, bytes data); + event AddProtectedToken(address indexed token); + event RemoveProtectedToken(address indexed token); event PresignHash(address indexed sender, bytes32 indexed hash); event SetDesignatedSigner(address indexed sender, address indexed oldSigner, address indexed newSigner); @@ -39,7 +66,7 @@ contract Agent is IERC165, ERC1271Bytes, IForwarder, IsContract, Vault { */ 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)))) // bytes4 casted as uint256 sets the bytes as the LSBs + authP(EXECUTE_ROLE, arr(_target, _ethValue, uint256(_getSig(_data)))) // bytes4 casted as uint256 sets the bytes as the LSBs { bool result = _target.call.value(_ethValue)(_data); @@ -48,17 +75,107 @@ contract Agent is IERC165, ERC1271Bytes, IForwarder, IsContract, Vault { } assembly { - let size := returndatasize let ptr := mload(0x40) - returndatacopy(ptr, 0, size) + returndatacopy(ptr, 0, returndatasize) // 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) } + switch result case 0 { revert(ptr, returndatasize) } + default { return(ptr, returndatasize) } + } + } + + /** + * @notice Execute '`@radspec(_target, _data)`' on `_target` ensuring that protected tokens can't be spent + * @param _target Address where the action is being executed + * @param _data Calldata for the action + * @return Exits call frame forwarding the return data of the executed call (either error or success data) + */ + function safeExecute(address _target, bytes _data) + external // This function MUST always be external as the function performs a low level return, exiting the Agent app execution context + authP(SAFE_EXECUTE_ROLE, arr(_target, uint256(_getSig(_data)))) // bytes4 casted as uint256 sets the bytes as the LSBs + { + uint256 protectedTokensLength = protectedTokens.length; + address[] memory protectedTokens_ = new address[](protectedTokensLength); + uint256[] memory balances = new uint256[](protectedTokensLength); + + for (uint256 i = 0; i < protectedTokensLength; i++) { + address token = protectedTokens[i]; + require(_target != token, ERROR_TARGET_PROTECTED); + // we copy the protected tokens array to check whether the storage array has been modified during the underlying call + protectedTokens_[i] = token; + // we copy the balances to check whether they have been modified during the underlying call + balances[i] = balance(token); + } + + bool result = _target.call(_data); + + bytes32 ptr; + uint256 size; + assembly { + size := returndatasize + ptr := mload(0x40) + mstore(0x40, add(ptr, returndatasize)) + returndatacopy(ptr, 0, returndatasize) + } + + if (result) { + // if the underlying call has succeeded, we check that the protected tokens + // and their balances have not been modified and return the call's return data + require(protectedTokens.length == protectedTokensLength, ERROR_PROTECTED_TOKENS_MODIFIED); + for (uint256 j = 0; j < protectedTokensLength; j++) { + require(protectedTokens[j] == protectedTokens_[j], ERROR_PROTECTED_TOKENS_MODIFIED); + require(balance(protectedTokens[j]) >= balances[j], ERROR_PROTECTED_BALANCE_LOWERED); + } + + emit SafeExecute(msg.sender, _target, _data); + + assembly { + return(ptr, size) + } + } else { + // if the underlying call has failed, we revert and forward returned error data + assembly { + revert(ptr, size) + } } } + /** + * @notice Add `_token.symbol(): string` to the list of protected tokens + * @param _token Address of the token to be protected + */ + function addProtectedToken(address _token) external authP(ADD_PROTECTED_TOKEN_ROLE, arr(_token)) { + require(protectedTokens.length < PROTECTED_TOKENS_CAP, ERROR_TOKENS_CAP_REACHED); + require(_isERC20(_token), ERROR_TOKEN_NOT_ERC20); + require(!_tokenIsProtected(_token), ERROR_TOKEN_ALREADY_PROTECTED); + + _addProtectedToken(_token); + } + + /** + * @notice Remove `_token.symbol(): string` from the list of protected tokens + * @param _token Address of the token to be unprotected + */ + function removeProtectedToken(address _token) external authP(REMOVE_PROTECTED_TOKEN_ROLE, arr(_token)) { + require(_tokenIsProtected(_token), ERROR_TOKEN_NOT_PROTECTED); + + _removeProtectedToken(_token); + } + + /** + * @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); + } + /** * @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 @@ -80,48 +197,67 @@ contract Agent is IERC165, ERC1271Bytes, IForwarder, IsContract, Vault { emit SetDesignatedSigner(msg.sender, oldDesignatedSigner, _designatedSigner); } + // Forwarding fns + /** - * @notice Pre-sign hash `_hash` - * @param _hash Hash that will be considered signed regardless of the signature checked with 'isValidSignature()' + * @notice Tells whether the Agent app is a forwarder or not + * @dev IForwarder interface conformance + * @return Always true */ - 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))) - { + function forward(bytes _evmScript) public { + require(canForward(msg.sender, _evmScript), ERROR_CAN_NOT_FORWARD); + 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) { + /** + * @notice Tells whether `_sender` can forward actions or not + * @dev IForwarder interface conformance + * @param _sender Address of the account intending to forward an action + * @return True if the given address can run scripts, false otherwise + */ + function canForward(address _sender, bytes _evmScript) public view returns (bool) { + // Note that `canPerform()` implicitly does an initialization check itself + return canPerform(_sender, RUN_SCRIPT_ROLE, arr(_getScriptACLParam(_evmScript))); + } + + // ERC-165 conformance + + /** + * @notice Tells whether this contract supports a given ERC-165 interface + * @param _interfaceId Interface bytes to check + * @return True if this contract supports the interface + */ + function supportsInterface(bytes4 _interfaceId) external pure returns (bool) { + return + _interfaceId == ERC1271_INTERFACE_ID || + _interfaceId == ERC165_INTERFACE_ID; + } + + // ERC-1271 conformance + + /** + * @notice Tells whether a signature is seen as valid by this contract through ERC-1271 + * @param _hash Arbitrary length data signed on the behalf of address (this) + * @param _signature Signature byte array associated with _data + * @return The ERC-1271 magic value if the signature is valid + */ + 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]) { + if (isPresigned[_hash]) { return returnIsValidSignatureMagicNumber(true); } @@ -129,27 +265,73 @@ contract Agent is IERC165, ERC1271Bytes, IForwarder, IsContract, Vault { if (designatedSigner == address(0)) { isValid = false; } else { - isValid = SignatureValidator.isValidSignature(hash, designatedSigner, signature); + 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); + // Getters + + function getProtectedTokensLength() public view isInitialized returns (uint256) { + return protectedTokens.length; + } + + // Internal fns + + function _addProtectedToken(address _token) internal { + protectedTokens.push(_token); + + emit AddProtectedToken(_token); + } + + function _removeProtectedToken(address _token) internal { + protectedTokens[_protectedTokenIndex(_token)] = protectedTokens[protectedTokens.length - 1]; + protectedTokens.length--; + + emit RemoveProtectedToken(_token); + } + + function _isERC20(address _token) internal view returns (bool) { + if (!isContract(_token)) { + return false; + } + + // Throwaway sanity check to make sure the token's `balanceOf()` does not error (for now) + balance(_token); + + return true; + } + + function _protectedTokenIndex(address _token) internal view returns (uint256) { + for (uint i = 0; i < protectedTokens.length; i++) { + if (protectedTokens[i] == _token) { + return i; + } + } + + revert(ERROR_TOKEN_NOT_PROTECTED); + } + + function _tokenIsProtected(address _token) internal view returns (bool) { + for (uint256 i = 0; i < protectedTokens.length; i++) { + if (protectedTokens[i] == _token) { + return true; + } + } + + return false; } - function getScriptACLParam(bytes evmScript) internal pure returns (uint256) { - return uint256(keccak256(abi.encodePacked(evmScript))); + function _getScriptACLParam(bytes _evmScript) internal pure returns (uint256) { + return uint256(keccak256(abi.encodePacked(_evmScript))); } - function getSig(bytes data) internal pure returns (bytes4 sig) { - if (data.length < 4) { + function _getSig(bytes _data) internal pure returns (bytes4 sig) { + if (_data.length < 4) { return; } - assembly { sig := mload(add(data, 0x20)) } + assembly { sig := mload(add(_data, 0x20)) } } } diff --git a/apps/agent/contracts/test/mocks/TokenInteractionExecutionTarget.sol b/apps/agent/contracts/test/mocks/TokenInteractionExecutionTarget.sol new file mode 100644 index 0000000000..b5d6ee9adb --- /dev/null +++ b/apps/agent/contracts/test/mocks/TokenInteractionExecutionTarget.sol @@ -0,0 +1,18 @@ +pragma solidity 0.4.24; + +import "@aragon/test-helpers/contracts/TokenMock.sol"; +import "../../Agent.sol"; + +contract TokenInteractionExecutionTarget { + function transferTokenFrom(address _token) external { + TokenMock(_token).transferFrom(msg.sender, this, 1); + } + + function transferTokenTo(address _token) external { + TokenMock(_token).transfer(msg.sender, 1); + } + + function removeProtectedToken(address _token) external { + Agent(msg.sender).removeProtectedToken(_token); + } +} diff --git a/apps/agent/test/agent_shared.js b/apps/agent/test/agent_shared.js index dcad6d105f..9878d46219 100644 --- a/apps/agent/test/agent_shared.js +++ b/apps/agent/test/agent_shared.js @@ -29,8 +29,11 @@ module.exports = ( const DesignatedSigner = artifacts.require('DesignatedSigner') const DestinationMock = artifacts.require('DestinationMock') const EtherTokenConstantMock = artifacts.require('EtherTokenConstantMock') + const TokenInteractionExecutionTarget = artifacts.require('TokenInteractionExecutionTarget') + const TokenMock = artifacts.require('TokenMock') const NO_SIG = '0x' + const NO_DATA = '0x' const ERC165_SUPPORT_INVALID_ID = '0xffffffff' const ERC165_SUPPORT_INTERFACE_ID = '0x01ffc9a7' @@ -39,7 +42,7 @@ module.exports = ( context(`> Shared tests for Agent-like apps`, () => { let daoFact, agentBase, dao, acl, agent, agentAppId - let ETH, ANY_ENTITY, APP_MANAGER_ROLE, EXECUTE_ROLE, RUN_SCRIPT_ROLE, ADD_PRESIGNED_HASH_ROLE, DESIGNATE_SIGNER_ROLE, ERC1271_INTERFACE_ID + let ETH, ANY_ENTITY, APP_MANAGER_ROLE, EXECUTE_ROLE, SAFE_EXECUTE_ROLE, RUN_SCRIPT_ROLE, ADD_PROTECTED_TOKEN_ROLE, REMOVE_PROTECTED_TOKEN_ROLE, ADD_PRESIGNED_HASH_ROLE, DESIGNATE_SIGNER_ROLE, ERC1271_INTERFACE_ID // Error strings const errors = makeErrorMappingProxy({ @@ -48,12 +51,23 @@ module.exports = ( INIT_ALREADY_INITIALIZED: 'INIT_ALREADY_INITIALIZED', INIT_NOT_INITIALIZED: 'INIT_NOT_INITIALIZED', RECOVER_DISALLOWED: 'RECOVER_DISALLOWED', + SAFE_ERC_20_BALANCE_REVERTED: 'SAFE_ERC_20_BALANCE_REVERTED', // Agent errors - AGENT_DESIGNATED_TO_SELF: "AGENT_DESIGNATED_TO_SELF", + AGENT_TARGET_PROTECTED: 'AGENT_TARGET_PROTECTED', + AGENT_PROTECTED_TOKENS_MODIFIED: 'AGENT_PROTECTED_TOKENS_MODIFIED', + AGENT_PROTECTED_BALANCE_LOWERED: 'AGENT_PROTECTED_BALANCE_LOWERED', + AGENT_TOKENS_CAP_REACHED: 'AGENT_TOKENS_CAP_REACHED', + AGENT_TOKEN_ALREADY_PROTECTED: 'AGENT_TOKEN_ALREADY_PROTECTED', + AGENT_TOKEN_NOT_ERC20: 'AGENT_TOKEN_NOT_ERC20', + AGENT_TOKEN_NOT_PROTECTED: 'AGENT_TOKEN_NOT_PROTECTED', + AGENT_DESIGNATED_TO_SELF: 'AGENT_DESIGNATED_TO_SELF', + AGENT_CAN_NOT_FORWARD: 'AGENT_CAN_NOT_FORWARD', }) const root = accounts[0] + const authorized = accounts[1] + const unauthorized = accounts[2] const encodeFunctionCall = (contract, functionName, ...params) => contract[functionName].request(...params).params[0] @@ -68,10 +82,13 @@ module.exports = ( // Setup constants ANY_ENTITY = await aclBase.ANY_ENTITY() APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE() + SAFE_EXECUTE_ROLE = await agentBase.SAFE_EXECUTE_ROLE() EXECUTE_ROLE = await agentBase.EXECUTE_ROLE() - RUN_SCRIPT_ROLE = await agentBase.RUN_SCRIPT_ROLE() + ADD_PROTECTED_TOKEN_ROLE = await agentBase.ADD_PROTECTED_TOKEN_ROLE() + REMOVE_PROTECTED_TOKEN_ROLE = await agentBase.REMOVE_PROTECTED_TOKEN_ROLE() ADD_PRESIGNED_HASH_ROLE = await agentBase.ADD_PRESIGNED_HASH_ROLE() DESIGNATE_SIGNER_ROLE = await agentBase.DESIGNATE_SIGNER_ROLE() + RUN_SCRIPT_ROLE = await agentBase.RUN_SCRIPT_ROLE() ERC1271_INTERFACE_ID = await agentBase.ERC1271_INTERFACE_ID() const ethConstant = await EtherTokenConstantMock.new() @@ -90,493 +107,893 @@ module.exports = ( const agentReceipt = await dao.newAppInstance(agentAppId, agentBase.address, '0x', false) agent = AgentLike.at(getNewProxyAddress(agentReceipt)) - - await agent.initialize() }) - context('> Executing actions', () => { - const [_, nonExecutor, executor] = accounts - let executionTarget - - for (const depositAmount of [0, 3]) { - context(depositAmount ? '> With ETH' : '> Without ETH', () => { - beforeEach(async () => { - await acl.createPermission(executor, agent.address, EXECUTE_ROLE, root, { from: root }) - - executionTarget = await ExecutionTarget.new() - assert.equal(await executionTarget.counter(), 0, 'expected starting counter of execution target to be be 0') - assert.equal((await getBalance(executionTarget.address)).toString(), 0, 'expected starting balance of execution target to be be 0') - - if (depositAmount) { - await agent.deposit(ETH, depositAmount, { value: depositAmount }) - } - assert.equal((await getBalance(agent.address)).toString(), depositAmount, `expected starting balance of agent to be ${depositAmount}`) - }) + context('> Uninitialized', () => { + it('cannot initialize base app', async () => { + const newAgent = await AgentLike.new() + assert.isTrue(await newAgent.isPetrified()) + await assertRevert(newAgent.initialize(), errors.INIT_ALREADY_INITIALIZED) + }) - it('can execute actions', async () => { - const N = 1102 + it('can be initialized', async () => { + await agent.initialize() + assert.isTrue(await agent.hasInitialized()) + }) - const data = executionTarget.contract.setCounter.getData(N) - const receipt = await agent.execute(executionTarget.address, depositAmount, data, { from: executor }) + it('cannot execute actions', async () => { + await acl.createPermission(root, agent.address, EXECUTE_ROLE, root, { from: root }) + await assertRevert(agent.execute(accounts[8], 0, NO_DATA), errors.APP_AUTH_FAILED) + }) - assertAmountOfEvents(receipt, 'Execute') - assert.equal(await executionTarget.counter(), N, `expected counter to be ${N}`) - assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') - }) + it('cannot safe execute actions', async () => { + await acl.createPermission(root, agent.address, SAFE_EXECUTE_ROLE, root, { from: root }) + await assertRevert(agent.safeExecute(accounts[8], NO_DATA), errors.APP_AUTH_FAILED) + }) - it('can execute actions without data', async () => { - const noData = '0x' - const receipt = await agent.execute(executionTarget.address, depositAmount, noData, { from: executor }) + it('cannot add protected tokens', async () => { + await acl.createPermission(root, agent.address, ADD_PROTECTED_TOKEN_ROLE, root, { from: root }) + await assertRevert(agent.addProtectedToken(accounts[8]), errors.APP_AUTH_FAILED) + }) - assertAmountOfEvents(receipt, 'Execute') - // Fallback just runs ExecutionTarget.execute() - assert.equal(await executionTarget.counter(), 1, 'expected counter to be 1') - assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') - }) + it('cannot remove protected tokens', async () => { + await acl.createPermission(root, agent.address, REMOVE_PROTECTED_TOKEN_ROLE, root, { from: root }) + await assertRevert(agent.removeProtectedToken(accounts[8]), errors.APP_AUTH_FAILED) + }) - it('can execute cheap fallback actions', async () => { - const cheapFallbackTarget = await DestinationMock.new(false) - const noData = '0x' - const receipt = await agent.execute(cheapFallbackTarget.address, depositAmount, noData, { from: executor }) + it('cannot add presigned hashes', async () => { + const hash = web3.sha3('hash') // careful as it may encode the data in the same way as solidity before hashing - assertAmountOfEvents(receipt, 'Execute') - assert.equal((await getBalance(cheapFallbackTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') - }) + await acl.createPermission(root, agent.address, ADD_PRESIGNED_HASH_ROLE, root, { from: root }) + await assertRevert(agent.presignHash(hash), errors.APP_AUTH_FAILED) + }) - it('can execute expensive fallback actions', async () => { - const expensiveFallbackTarget = await DestinationMock.new(true) - assert.equal(await expensiveFallbackTarget.counter(), 0) + it('cannot set designated signers', async () => { + await acl.createPermission(root, agent.address, DESIGNATE_SIGNER_ROLE, root, { from: root }) + await assertRevert(agent.setDesignatedSigner(accounts[8]), errors.APP_AUTH_FAILED) + }) - const noData = '0x' - const receipt = await agent.execute(expensiveFallbackTarget.address, depositAmount, noData, { from: executor }) + it('cannot forward actions', async () => { + const action = { to: agent.address, calldata: agent.contract.presignHash.getData(accounts[8]) } + const script = encodeCallScript([action]) - assertAmountOfEvents(receipt, 'Execute') - // Fallback increments counter - assert.equal(await expensiveFallbackTarget.counter(), 1) - assert.equal((await getBalance(expensiveFallbackTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') - }) + await acl.createPermission(root, agent.address, RUN_SCRIPT_ROLE, root, { from: root }) + await assertRevert(agent.forward(script), errors.AGENT_CAN_NOT_FORWARD) + }) + }) - it('can execute with data when target is not a contract', async () => { - const nonContract = accounts[8] // random account - const nonContractBalance = await getBalance(nonContract) - const randomData = '0x12345678' + context('> Initialized', () => { + beforeEach(async () => { + await agent.initialize() + }) - const receipt = await agent.execute(nonContract, depositAmount, randomData, { from: executor }) + it('fails on reinitialization', async () => { + await assertRevert(agent.initialize(), errors.INIT_ALREADY_INITIALIZED) + }) - assertAmountOfEvents(receipt, 'Execute') - assert.equal((await getBalance(nonContract)).toString(), nonContractBalance.add(depositAmount).toString(), 'expected ending balance of non-contract to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') - }) + context('> Executing actions', () => { + const [_, nonExecutor, executor] = accounts + let executionTarget - it('can execute without data when target is not a contract', async () => { - const nonContract = accounts[8] // random account - const nonContractBalance = await getBalance(nonContract) - const noData = '0x' + for (const depositAmount of [0, 3]) { + context(depositAmount ? '> With ETH' : '> Without ETH', () => { + beforeEach(async () => { + await acl.createPermission(executor, agent.address, EXECUTE_ROLE, root, { from: root }) - const receipt = await agent.execute(nonContract, depositAmount, noData, { from: executor }) + executionTarget = await ExecutionTarget.new() + assert.equal(await executionTarget.counter(), 0, 'expected starting counter of execution target to be be 0') + assert.equal((await getBalance(executionTarget.address)).toString(), 0, 'expected starting balance of execution target to be be 0') - assertAmountOfEvents(receipt, 'Execute') - assert.equal((await getBalance(nonContract)).toString(), nonContractBalance.add(depositAmount).toString(), 'expected ending balance of non-contract to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') - }) + if (depositAmount) { + await agent.deposit(ETH, depositAmount, { value: depositAmount }) + } + assert.equal((await getBalance(agent.address)).toString(), depositAmount, `expected starting balance of agent to be ${depositAmount}`) + }) - it('fails to execute without permissions', async () => { - const data = executionTarget.contract.execute.getData() + it('can execute actions', async () => { + const N = 1102 - await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: nonExecutor }), errors.APP_AUTH_FAILED) - }) + const data = executionTarget.contract.setCounter.getData(N) + const receipt = await agent.execute(executionTarget.address, depositAmount, data, { from: executor }) - it('fails to execute actions with more ETH than the agent owns', async () => { - const data = executionTarget.contract.execute.getData() + assertAmountOfEvents(receipt, 'Execute') + assert.equal(await executionTarget.counter(), N, `expected counter to be ${N}`) + assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + }) - await assertRevert(agent.execute(executionTarget.address, depositAmount + 1, data, { from: executor })) - }) + it('can execute actions without data', async () => { + const receipt = await agent.execute(executionTarget.address, depositAmount, NO_DATA, { from: executor }) - it('execution forwards success return data', async () => { - const { to, data } = encodeFunctionCall(executionTarget, 'execute') + assertAmountOfEvents(receipt, 'Execute') + // Fallback just runs ExecutionTarget.execute() + assert.equal(await executionTarget.counter(), 1, 'expected counter to be 1') + assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + }) - // We make a call to easily get what data could be gotten inside the EVM - // Contract -> agent.execute -> Target.func (would allow Contract to have access to this data) - const call = encodeFunctionCall(agent, 'execute', to, depositAmount, data, { from: executor }) - const returnData = await web3Call(call) + it('can execute cheap fallback actions', async () => { + const cheapFallbackTarget = await DestinationMock.new(false) + const receipt = await agent.execute(cheapFallbackTarget.address, depositAmount, NO_DATA, { from: executor }) - // ExecutionTarget.execute() increments the counter by 1 - assert.equal(ethABI.decodeParameter('uint256', returnData), 1) - }) + assertAmountOfEvents(receipt, 'Execute') + assert.equal((await getBalance(cheapFallbackTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + }) - it('it reverts if executed action reverts', async () => { - // TODO: Check revert data was correctly forwarded - // ganache currently doesn't support fetching this data + it('can execute expensive fallback actions', async () => { + const expensiveFallbackTarget = await DestinationMock.new(true) + assert.equal(await expensiveFallbackTarget.counter(), 0) - const data = executionTarget.contract.fail.getData() - await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: executor })) - }) + const receipt = await agent.execute(expensiveFallbackTarget.address, depositAmount, NO_DATA, { from: executor }) - context('depending on the sig ACL param', () => { - const [ granteeEqualToSig, granteeUnequalToSig ] = accounts.slice(6) // random slice from accounts + assertAmountOfEvents(receipt, 'Execute') + // Fallback increments counter + assert.equal(await expensiveFallbackTarget.counter(), 1) + assert.equal((await getBalance(expensiveFallbackTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + }) - beforeEach(async () => { - const sig = executionTarget.contract.setCounter.getData(1).slice(2, 10) - const argId = '0x02' // arg 2 - const equalOp = '01' - const nonEqualOp = '02' - const value = `${'00'.repeat(30 - 4)}${sig}` + it('can execute with data when target is not a contract', async () => { + const nonContract = accounts[8] // random account + const nonContractBalance = await getBalance(nonContract) + const randomData = '0x12345678' - const equalParam = new web3.BigNumber(`${argId}${equalOp}${value}`) - const nonEqualParam = new web3.BigNumber(`${argId}${nonEqualOp}${value}`) + const receipt = await agent.execute(nonContract, depositAmount, randomData, { from: executor }) - await acl.grantPermissionP(granteeEqualToSig, agent.address, EXECUTE_ROLE, [equalParam], { from: root }) - await acl.grantPermissionP(granteeUnequalToSig, agent.address, EXECUTE_ROLE, [nonEqualParam], { from: root }) + assertAmountOfEvents(receipt, 'Execute') + assert.equal((await getBalance(nonContract)).toString(), nonContractBalance.add(depositAmount).toString(), 'expected ending balance of non-contract to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') }) - it('equal param: can execute if the signature matches', async () => { - const N = 1102 + it('can execute without data when target is not a contract', async () => { + const nonContract = accounts[8] // random account + const nonContractBalance = await getBalance(nonContract) - const data = executionTarget.contract.setCounter.getData(N) - const receipt = await agent.execute(executionTarget.address, depositAmount, data, { from: granteeEqualToSig }) + const receipt = await agent.execute(nonContract, depositAmount, NO_DATA, { from: executor }) assertAmountOfEvents(receipt, 'Execute') - assert.equal(await executionTarget.counter(), N, `expected counter to be ${N}`) - assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(nonContract)).toString(), nonContractBalance.add(depositAmount).toString(), 'expected ending balance of non-contract to be correct') assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') }) - it('not equal param: can execute if the signature doesn\'t match', async () => { + it('fails to execute without permissions', async () => { const data = executionTarget.contract.execute.getData() - const receipt = await agent.execute(executionTarget.address, depositAmount, data, { from: granteeUnequalToSig }) - assertAmountOfEvents(receipt, 'Execute') - assert.equal(await executionTarget.counter(), 1, `expected counter to be ${1}`) - assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') - assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: nonExecutor }), errors.APP_AUTH_FAILED) }) - it('equal param: fails to execute if signature doesn\'t match', async () => { + it('fails to execute actions with more ETH than the agent owns', async () => { const data = executionTarget.contract.execute.getData() - await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: granteeEqualToSig }), errors.APP_AUTH_FAILED) + await assertRevert(agent.execute(executionTarget.address, depositAmount + 1, data, { from: executor })) }) - it('not equal param: fails to execute if the signature matches', async () => { - const N = 1102 + it('execution forwards success return data', async () => { + const { to, data } = encodeFunctionCall(executionTarget, 'execute') - const data = executionTarget.contract.setCounter.getData(N) - await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: granteeUnequalToSig }), errors.APP_AUTH_FAILED) + // We make a call to easily get what data could be gotten inside the EVM + // Contract -> agent.execute -> Target.func (would allow Contract to have access to this data) + const call = encodeFunctionCall(agent, 'execute', to, depositAmount, data, { from: executor }) + const returnData = await web3Call(call) + + // ExecutionTarget.execute() increments the counter by 1 + assert.equal(ethABI.decodeParameter('uint256', returnData), 1) }) - }) - }) - } - }) - context('> Running scripts', () => { - let executionTarget, script - const [_, nonScriptRunner, scriptRunner] = accounts + it('it reverts if executed action reverts', async () => { + // TODO: Check revert data was correctly forwarded + // ganache currently doesn't support fetching this data - beforeEach(async () => { - executionTarget = await ExecutionTarget.new() - // prepare script - const action = { to: executionTarget.address, calldata: executionTarget.contract.execute.getData() } - script = encodeCallScript([action, action]) // perform action twice + const data = executionTarget.contract.fail.getData() + await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: executor })) + }) - await acl.createPermission(scriptRunner, agent.address, RUN_SCRIPT_ROLE, root, { from: root }) - }) + context('depending on the sig ACL param', () => { + const [granteeEqualToSig, granteeUnequalToSig] = accounts.slice(6) // random slice from accounts - it('runs script', async () => { - assert.isTrue(await agent.canForward(scriptRunner, script)) - assert.equal(await executionTarget.counter(), 0) + beforeEach(async () => { + const sig = executionTarget.contract.setCounter.getData(1).slice(2, 10) + const argId = '0x02' // arg 2 + const equalOp = '01' + const nonEqualOp = '02' + const value = `${'00'.repeat(30 - 4)}${sig}` - const receipt = await agent.forward(script, { from: scriptRunner }) + const equalParam = new web3.BigNumber(`${argId}${equalOp}${value}`) + const nonEqualParam = new web3.BigNumber(`${argId}${nonEqualOp}${value}`) - // Should execute ExecutionTarget.execute() twice - assert.equal(await executionTarget.counter(), 2) - assertAmountOfEvents(receipt, 'ScriptResult') - }) + await acl.grantPermissionP(granteeEqualToSig, agent.address, EXECUTE_ROLE, [equalParam], { from: root }) + await acl.grantPermissionP(granteeUnequalToSig, agent.address, EXECUTE_ROLE, [nonEqualParam], { from: root }) + }) - it('fails to run script without permissions', async () => { - assert.isFalse(await agent.canForward(nonScriptRunner, script)) - assert.equal(await executionTarget.counter(), 0) + it('equal param: can execute if the signature matches', async () => { + const N = 1102 - await assertRevert(agent.forward(script, { from: nonScriptRunner }), errors.APP_AUTH_FAILED) - assert.equal(await executionTarget.counter(), 0) - }) - }) + const data = executionTarget.contract.setCounter.getData(N) + const receipt = await agent.execute(executionTarget.address, depositAmount, data, { from: granteeEqualToSig }) - context('> Signing messages', () => { - const [_, nobody, presigner, signerDesignator] = accounts - const HASH = web3.sha3('hash') // careful as it may encode the data in the same way as solidity before hashing + assertAmountOfEvents(receipt, 'Execute') + assert.equal(await executionTarget.counter(), N, `expected counter to be ${N}`) + assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + }) - const SIGNATURE_MODES = { - Invalid: '0x00', - EIP712: '0x01', - EthSign: '0x02', - ERC1271: '0x03', - NMode: '0x04', - } + it('not equal param: can execute if the signature doesn\'t match', async () => { + const data = executionTarget.contract.execute.getData() + const receipt = await agent.execute(executionTarget.address, depositAmount, data, { from: granteeUnequalToSig }) - const ERC1271_RETURN_VALID_SIGNATURE = '0x20c13b0b' - const ERC1271_RETURN_INVALID_SIGNATURE = '0x00000000' + assertAmountOfEvents(receipt, 'Execute') + assert.equal(await executionTarget.counter(), 1, `expected counter to be ${1}`) + assert.equal((await getBalance(executionTarget.address)).toString(), depositAmount, 'expected ending balance of execution target to be correct') + assert.equal((await getBalance(agent.address)).toString(), 0, 'expected ending balance of agent at end to be 0') + }) - const assertIsValidSignature = (isValid, erc1271Return) => { - const expectedReturn = - isValid - ? ERC1271_RETURN_VALID_SIGNATURE - : ERC1271_RETURN_INVALID_SIGNATURE + it('equal param: fails to execute if signature doesn\'t match', async () => { + const data = executionTarget.contract.execute.getData() - assert.equal(erc1271Return, expectedReturn, `Expected signature to be ${isValid ? '' : 'in'}valid (returned ${erc1271Return})`) - } + await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: granteeEqualToSig }), errors.APP_AUTH_FAILED) + }) - beforeEach(async () => { - await acl.createPermission(presigner, agent.address, ADD_PRESIGNED_HASH_ROLE, root, { from: root }) - await acl.createPermission(signerDesignator, agent.address, DESIGNATE_SIGNER_ROLE, root, { from: root }) - }) + it('not equal param: fails to execute if the signature matches', async () => { + const N = 1102 - it('complies with ERC165', async () => { - assert.isTrue(await agent.supportsInterface(ERC165_SUPPORT_INTERFACE_ID)) - assert.isFalse(await agent.supportsInterface(ERC165_SUPPORT_INVALID_ID)) + const data = executionTarget.contract.setCounter.getData(N) + await assertRevert(agent.execute(executionTarget.address, depositAmount, data, { from: granteeUnequalToSig }), errors.APP_AUTH_FAILED) + }) + }) + }) + } }) - it('supports ERC1271 interface', async () => { - assert.isTrue(await agent.supportsInterface(ERC1271_INTERFACE_ID)) - }) + context('> Safe executing actions', () => { + const amount = 1000 + let target, token1, token2 - it('doesn\'t support any other interface', async () => { - assert.isFalse(await agent.supportsInterface('0x12345678')) - assert.isFalse(await agent.supportsInterface('0x')) - }) + beforeEach(async () => { + target = await ExecutionTarget.new() + token1 = await TokenMock.new(agent.address, amount) + token2 = await TokenMock.new(agent.address, amount) - it('isValidSignature returns false if there is not designated signer and hash isn\'t presigned', async () => { - assertIsValidSignature(false, await agent.isValidSignature(HASH, NO_SIG)) - }) + await acl.createPermission(authorized, agent.address, ADD_PROTECTED_TOKEN_ROLE, root, { from: root }) + await acl.createPermission(authorized, agent.address, REMOVE_PROTECTED_TOKEN_ROLE, root, { from: root }) + await acl.createPermission(authorized, agent.address, SAFE_EXECUTE_ROLE, root, { from: root }) - it('presigns a hash', async () => { - await agent.presignHash(HASH, { from: presigner }) + await agent.addProtectedToken(token1.address, { from: authorized }) + await agent.addProtectedToken(token2.address, { from: authorized }) - assertIsValidSignature(true, await agent.isValidSignature(HASH, NO_SIG)) - }) + assert.equal(await target.counter(), 0) + assert.equal(await token1.balanceOf(agent.address), amount) + assert.equal(await token2.balanceOf(agent.address), amount) + }) - it('fails to presign a hash if not authorized', async () => { - await assertRevert(agent.presignHash(HASH, { from: nobody }), errors.APP_AUTH_FAILED) - assertIsValidSignature(false, await agent.isValidSignature(HASH, NO_SIG)) - }) + context('> sender has SAFE_EXECUTE_ROLE', () => { + context('> and target is not a protected ERC20', () => { + it('it can execute actions', async () => { + const N = 1102 + const data = target.contract.setCounter.getData(N) + const receipt = await agent.safeExecute(target.address, data, { from: authorized }) - context('> Designated signer', () => { - const ethSign = async (hash, signer) => { - const packedSig = await web3Sign(signer, hash) + assertAmountOfEvents(receipt, 'SafeExecute') + assert.equal(await target.counter(), N) + }) - return { - r: ethUtil.toBuffer('0x' + packedSig.substring(2, 66)), - s: ethUtil.toBuffer('0x' + packedSig.substring(66, 130)), - v: parseInt(packedSig.substring(130, 132), 16) + 27, - mode: ethUtil.toBuffer(SIGNATURE_MODES.EthSign) - } - } + it('it can execute actions without data', async () => { + const receipt = await agent.safeExecute(target.address, NO_DATA, { from: authorized }) - const eip712Sign = async (hash, key) => ({ - mode: ethUtil.toBuffer(SIGNATURE_MODES.EIP712), - ...ethUtil.ecsign( - Buffer.from(hash.slice(2), 'hex'), - Buffer.from(key, 'hex') - ) - }) + assertAmountOfEvents(receipt, 'SafeExecute') + assert.equal(await target.counter(), 1) // fallback just runs ExecutionTarget.execute() + }) - const signFunctionGenerator = (signFunction, signatureModifier) => ( - async (hash, signerOrKey, useLegacySig = false, useInvalidV = false) => { - const sig = await signFunction(hash, signerOrKey) - const v = - useInvalidV - ? ethUtil.toBuffer(2) // force set an invalid v - : ethUtil.toBuffer(sig.v - (useLegacySig ? 0 : 27)) + it('it can execute cheap fallback actions', async () => { + const cheapFallbackTarget = await DestinationMock.new(false) + const receipt = await agent.safeExecute(cheapFallbackTarget.address, NO_DATA, { from: authorized }) - const signature = '0x' + Buffer.concat([sig.mode, sig.r, sig.s, v]).toString('hex') - return signatureModifier(signature) - } - ) + assertAmountOfEvents(receipt, 'SafeExecute') + }) - const addERC1271ModePrefix = (signature) => - `${SIGNATURE_MODES.ERC1271}${signature.slice(2)}` + it('it can execute expensive fallback actions', async () => { + const expensiveFallbackTarget = await DestinationMock.new(true) + assert.equal(await expensiveFallbackTarget.counter(), 0) + const receipt = await agent.safeExecute(expensiveFallbackTarget.address, NO_DATA, { from: authorized }) - const createChildAgentGenerator = (designatedSigner) => - async () => { - const agentReceipt = await dao.newAppInstance(agentAppId, agentBase.address, '0x', false) - const childAgent = AgentLike.at(getNewProxyAddress(agentReceipt)) + assertAmountOfEvents(receipt, 'SafeExecute') + assert.equal(await expensiveFallbackTarget.counter(), 1) // fallback increments counter + }) - await childAgent.initialize() - await acl.createPermission(signerDesignator, childAgent.address, DESIGNATE_SIGNER_ROLE, root, { from: root }) - await childAgent.setDesignatedSigner(designatedSigner, { from: signerDesignator }) + it('it can execute with data when target is not a contract', async () => { + const nonContract = accounts[8] // random account + const randomData = '0x12345678' + const receipt = await agent.safeExecute(nonContract, randomData, { from: authorized }) - return childAgent.address - } + assertAmountOfEvents(receipt, 'SafeExecute') + }) - const directSignatureTests = [ - { - name: 'EIP712', - signFunction: eip712Sign, - getSigner: () => '0x93070b307c373D7f9344859E909e3EEeF6E4Fd5a', - signerOrKey: '11bc31e7fef59610dfd6f95d2f78d2396c7b5477e4a9a54d72d9c1b76930e5c1', - notSignerOrKey: '7224b5bc510e01f75b10e3b6d6c903861ca91adb95a26406d1603e2d28a29e7f', - }, - { - name: 'EthSign', - signFunction: ethSign, - getSigner: () => accounts[7], - signerOrKey: accounts[7], - notSignerOrKey: accounts[8] - }, - ] - - const wrappedSignatureTests = directSignatureTests.map(signatureTest => ({ - ...signatureTest, - name: `ERC1271 -> ${signatureTest.name}`, - signatureModifier: addERC1271ModePrefix, - getSigner: createChildAgentGenerator(signatureTest.getSigner()), - })) - - const signatureTests = directSignatureTests.concat(wrappedSignatureTests) - - for (const { - name, - signFunction, - getSigner, - signerOrKey, - notSignerOrKey, - signatureModifier = sig => sig // defaults to identity function (returns input) - } of signatureTests) { - const sign = signFunctionGenerator(signFunction, signatureModifier) - - context(`> Signature mode: ${name}`, () => { - beforeEach(async () => { - const signer = await getSigner() - await agent.setDesignatedSigner(signer, { from: signerDesignator }) + it('it can execute without data when target is not a contract', async () => { + const nonContract = accounts[8] // random account + const receipt = await agent.safeExecute(nonContract, NO_DATA, { from: authorized }) + + assertAmountOfEvents(receipt, 'SafeExecute') }) - it('isValidSignature returns true to a valid signature', async () => { - const signature = await sign(HASH, signerOrKey) - assertIsValidSignature(true, await agent.isValidSignature(HASH, signature)) + it('it can forward success return data', async () => { + const { to, data } = encodeFunctionCall(target, 'execute') + + // We make a call to easily get what data could be gotten inside the EVM + // Contract -> agent.safeExecute -> Target.func (would allow Contract to have access to this data) + const call = encodeFunctionCall(agent, 'safeExecute', to, data, { from: authorized }) + const returnData = await web3Call(call) + + // ExecutionTarget.execute() increments the counter by 1 + assert.equal(ethABI.decodeParameter('uint256', returnData), 1) }) - it('isValidSignature returns true to a valid signature with legacy version', async () => { - const legacyVersionSignature = await sign(HASH, signerOrKey, true) - assertIsValidSignature(true, await agent.isValidSignature(HASH, legacyVersionSignature)) + it('it should revert if executed action reverts', async () => { + // TODO: Check revert data was correctly forwarded + // ganache currently doesn't support fetching this data + const data = target.contract.fail.getData() + await assertRevert(agent.safeExecute(target.address, data, { from: authorized })) }) - it('isValidSignature returns false to an invalid signature', async () => { - const badSignature = (await sign(HASH, signerOrKey)).slice(0, -2) // drop last byte - assertIsValidSignature(false, await agent.isValidSignature(HASH, badSignature)) + context('> but action affects a protected ERC20 balance', () => { + let tokenInteractionTarget + + beforeEach(async () => { + tokenInteractionTarget = await TokenInteractionExecutionTarget.new() + }) + + context('> action decreases protected ERC20 balance', () => { + it('it should revert', async () => { + const newToken = await TokenMock.new(agent.address, amount) + const initialBalance = (await newToken.balanceOf(agent.address)).toNumber() + + const approve = newToken.contract.approve.getData(tokenInteractionTarget.address, 10) + await agent.safeExecute(newToken.address, approve, { from: authorized }) // target is now allowed to transfer on behalf of agent + + await agent.addProtectedToken(newToken.address, { from: authorized }) // new token is now protected (must do this after execution) + const data = tokenInteractionTarget.contract.transferTokenFrom.getData(newToken.address) + + await assertRevert(agent.safeExecute(tokenInteractionTarget.address, data, { from: authorized }), errors.AGENT_PROTECTED_BALANCE_LOWERED) + const newBalance = (await newToken.balanceOf(agent.address)).toNumber() + + assert.equal(initialBalance, newBalance) + }) + }) + + context('> action increases protected ERC20 balance', () => { + it('it should execute action', async () => { + const newToken = await TokenMock.new(tokenInteractionTarget.address, amount) + const initialBalance = (await newToken.balanceOf(agent.address)).toNumber() + await agent.addProtectedToken(newToken.address, { from: authorized }) // newToken is now protected + + const data = tokenInteractionTarget.contract.transferTokenTo.getData(newToken.address) + await agent.safeExecute(tokenInteractionTarget.address, data, { from: authorized }) + const newBalance = (await newToken.balanceOf(agent.address)).toNumber() + + assert.equal(newBalance, 1) + assert.notEqual(initialBalance, newBalance) + }) + }) }) - it('isValidSignature returns false to a signature with an invalid v', async () => { - const invalidVersionSignature = await sign(HASH, signerOrKey, false, true) - assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidVersionSignature)) + context('> but action affects protected tokens list', () => { + let tokenInteractionTarget + + beforeEach(async () => { + tokenInteractionTarget = await TokenInteractionExecutionTarget.new() + }) + + it('it should revert', async () => { + await acl.grantPermission(tokenInteractionTarget.address, agent.address, REMOVE_PROTECTED_TOKEN_ROLE, { from: root }) // grant target permission to remove protected tokens + const data = tokenInteractionTarget.contract.removeProtectedToken.getData(token2.address) + + await assertRevert(agent.safeExecute(tokenInteractionTarget.address, data, { from: authorized }), errors.AGENT_PROTECTED_TOKENS_MODIFIED) + }) }) + }) - it('isValidSignature returns false to an unauthorized signer', async () => { - const otherSignature = await sign(HASH, notSignerOrKey) - assertIsValidSignature(false, await agent.isValidSignature(HASH, otherSignature)) + context('> but target is a protected ERC20', () => { + it('it should revert', async () => { + const approve = token1.contract.approve.getData(target.address, 10) + + await assertRevert(agent.safeExecute(token1.address, approve, { from: authorized }), errors.AGENT_TARGET_PROTECTED) }) }) - } + }) - context('> Signature mode: ERC1271', () => { - const ERC1271_SIG = SIGNATURE_MODES.ERC1271 + context('> sender does not have SAFE_EXECUTE_ROLE', () => { + it('it should revert', async () => { + const data = target.contract.execute.getData() - const setDesignatedSignerContract = async (...params) => { - const designatedSigner = await DesignatedSigner.new(...params) - return agent.setDesignatedSigner(designatedSigner.address, { from: signerDesignator }) - } + await assertRevert(agent.safeExecute(target.address, data, { from: unauthorized }), errors.APP_AUTH_FAILED) + }) + }) + }) + + context('> Running scripts', () => { + let executionTarget, script + const [_, nonScriptRunner, scriptRunner] = accounts + + beforeEach(async () => { + executionTarget = await ExecutionTarget.new() + // prepare script + const action = { to: executionTarget.address, calldata: executionTarget.contract.execute.getData() } + script = encodeCallScript([action, action]) // perform action twice + + await acl.createPermission(scriptRunner, agent.address, RUN_SCRIPT_ROLE, root, { from: root }) + }) + + it('runs script', async () => { + assert.isTrue(await agent.canForward(scriptRunner, script)) + assert.equal(await executionTarget.counter(), 0) + + const receipt = await agent.forward(script, { from: scriptRunner }) - it('isValidSignature returns true if designated signer returns true', async () => { - // true - ERC165 interface compliant - // true - any sigs are valid - // false - doesn't revert on checking sig - // false - doesn't modify state on checking sig - await setDesignatedSignerContract(true, true, false, false) + // Should execute ExecutionTarget.execute() twice + assert.equal(await executionTarget.counter(), 2) + assertAmountOfEvents(receipt, 'ScriptResult') + }) + + it('fails to run script without permissions', async () => { + assert.isFalse(await agent.canForward(nonScriptRunner, script)) + assert.equal(await executionTarget.counter(), 0) + + await assertRevert(agent.forward(script, { from: nonScriptRunner }), errors.AGENT_CAN_NOT_FORWARD) + assert.equal(await executionTarget.counter(), 0) + }) + }) + + context('> Adding protected tokens', () => { + beforeEach(async () => { + await acl.createPermission(authorized, agent.address, ADD_PROTECTED_TOKEN_ROLE, root, { from: root }) + }) - assertIsValidSignature(true, await agent.isValidSignature(HASH, ERC1271_SIG)) + context('> sender has ADD_PROTECTED_TOKEN_ROLE', () => { + context('> and token is ERC20', () => { + context('> and token is not already protected', () => { + context('> and protected tokens cap has not yet been reached', () => { + it('it should add protected token', async () => { + const token1 = await TokenMock.new(agent.address, 10000) + const token2 = await TokenMock.new(agent.address, 10000) + + const receipt1 = await agent.addProtectedToken(token1.address, { from: authorized }) + const receipt2 = await agent.addProtectedToken(token2.address, { from: authorized }) + + assertAmountOfEvents(receipt1, 'AddProtectedToken') + assertAmountOfEvents(receipt2, 'AddProtectedToken') + assert.equal(await agent.protectedTokens(0), token1.address) + assert.equal(await agent.protectedTokens(1), token2.address) + }) + }) + + context('> but protected tokens cap has been reached', () => { + beforeEach(async () => { + const token1 = await TokenMock.new(agent.address, 1000) + const token2 = await TokenMock.new(agent.address, 1000) + const token3 = await TokenMock.new(agent.address, 1000) + const token4 = await TokenMock.new(agent.address, 1000) + const token5 = await TokenMock.new(agent.address, 1000) + const token6 = await TokenMock.new(agent.address, 1000) + const token7 = await TokenMock.new(agent.address, 1000) + const token8 = await TokenMock.new(agent.address, 1000) + const token9 = await TokenMock.new(agent.address, 1000) + const token10 = await TokenMock.new(agent.address, 1000) + + await agent.addProtectedToken(token1.address, { from: authorized }) + await agent.addProtectedToken(token2.address, { from: authorized }) + await agent.addProtectedToken(token3.address, { from: authorized }) + await agent.addProtectedToken(token4.address, { from: authorized }) + await agent.addProtectedToken(token5.address, { from: authorized }) + await agent.addProtectedToken(token6.address, { from: authorized }) + await agent.addProtectedToken(token7.address, { from: authorized }) + await agent.addProtectedToken(token8.address, { from: authorized }) + await agent.addProtectedToken(token9.address, { from: authorized }) + await agent.addProtectedToken(token10.address, { from: authorized }) + }) + + it('it should revert', async () => { + const token10 = await TokenMock.new(agent.address, 10000) + + await assertRevert(agent.addProtectedToken(token10.address, { from: authorized }), errors.AGENT_TOKENS_CAP_REACHED) + }) + }) + }) + + context('> but token is already protected', () => { + it('it should revert', async () => { + const token = await TokenMock.new(agent.address, 10000) + await agent.addProtectedToken(token.address, { from: authorized }) + + await assertRevert(agent.addProtectedToken(token.address, { from: authorized }), errors.AGENT_TOKEN_ALREADY_PROTECTED) + }) + }) }) - it('isValidSignature returns false if designated signer returns false', async () => { - // true - ERC165 interface compliant - // false - sigs are invalid - // false - doesn't revert on checking sig - // false - doesn't modify state on checking sig - await setDesignatedSignerContract(true, false, false, false) + context('> but token is not ERC20', () => { + it('it should revert [token is not a contract]', async () => { + await assertRevert(agent.addProtectedToken(root, { from: authorized }), errors.AGENT_TOKEN_NOT_ERC20) + }) - // Signature fails check - assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG)) + it('it should revert [token is a contract but not an ERC20]', async () => { + // The balanceOf check reverts here, so it's a SafeERC20 error + await assertRevert(agent.addProtectedToken(daoFact.address, { from: authorized }), errors.SAFE_ERC_20_BALANCE_REVERTED) + }) }) + }) - it('isValidSignature returns true even if the designated signer doesnt support the interface', async () => { - // false - not ERC165 interface compliant - // true - any sigs are valid - // false - doesn't revert on checking sig - // false - doesn't modify state on checking sig - await setDesignatedSignerContract(false, true, false, false) + context('> sender does not have ADD_PROTECTED_TOKEN_ROLE', () => { + it('it should revert', async () => { + const token = await TokenMock.new(agent.address, 10000) - assertIsValidSignature(true, await agent.isValidSignature(HASH, ERC1271_SIG)) + await assertRevert(agent.addProtectedToken(token.address, { from: unauthorized }), errors.APP_AUTH_FAILED) }) + }) + }) + + context('> Removing protected tokens', () => { + beforeEach(async () => { + await acl.createPermission(authorized, agent.address, ADD_PROTECTED_TOKEN_ROLE, root, { from: root }) + await acl.createPermission(authorized, agent.address, REMOVE_PROTECTED_TOKEN_ROLE, root, { from: root }) + }) + + context('> sender has REMOVE_PROTECTED_TOKEN_ROLE', () => { + context('> and token is actually protected', () => { + let token1, token2, token3 + + beforeEach(async () => { + token1 = await TokenMock.new(agent.address, 10000) + token2 = await TokenMock.new(agent.address, 10000) + token3 = await TokenMock.new(agent.address, 10000) + + await agent.addProtectedToken(token1.address, { from: authorized }) + await agent.addProtectedToken(token2.address, { from: authorized }) + await agent.addProtectedToken(token3.address, { from: authorized }) + }) - it('isValidSignature returns false if designated signer reverts', async () => { - // true - ERC165 interface compliant - // true - any sigs are valid - // true - reverts on checking sig - // false - doesn't modify state on checking sig - await setDesignatedSignerContract(true, true, true, false) + it('it should remove protected token', async () => { + const receipt1 = await agent.removeProtectedToken(token1.address, { from: authorized }) + const receipt2 = await agent.removeProtectedToken(token3.address, { from: authorized }) - // Reverts on checking - assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG)) + assertAmountOfEvents(receipt1, 'RemoveProtectedToken') + assertAmountOfEvents(receipt2, 'RemoveProtectedToken') + + assert.equal(await agent.getProtectedTokensLength(), 1) + assert.equal(await agent.protectedTokens(0), token2.address) + await assertRevert(agent.protectedTokens(1)) // this should overflow the length of the protectedTokens array and thus revert + }) }) - it('isValidSignature returns false if designated signer attempts to modify state', async () => { - // true - ERC165 interface compliant - // true - any sigs are valid - // false - doesn't revert on checking sig - // true - modifies state on checking sig - await setDesignatedSignerContract(true, true, false, true) + context('> but token is not actually protected', () => { + it('it should revert', async () => { + const token1 = await TokenMock.new(agent.address, 10000) + const token2 = await TokenMock.new(agent.address, 10000) + await agent.addProtectedToken(token1.address, { from: authorized }) - // Checking costs too much gas - assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG)) + await assertRevert(agent.removeProtectedToken(token2.address, { from: authorized }), errors.AGENT_TOKEN_NOT_PROTECTED) + }) }) }) - context(`> Signature mode: invalid modes`, () => { - const randomAccount = accounts[9] + context('> sender does not have REMOVE_PROTECTED_TOKEN_ROLE', () => { + it('it should revert', async () => { + const token = await TokenMock.new(agent.address, 10000) + await agent.addProtectedToken(token.address, { from: authorized }) + + await assertRevert(agent.removeProtectedToken(token.address, { from: unauthorized }), errors.APP_AUTH_FAILED) + }) + }) + }) + + context('> Accessing protected tokens', () => { + beforeEach(async () => { + await acl.createPermission(authorized, agent.address, ADD_PROTECTED_TOKEN_ROLE, root, { from: root }) + }) + + context('> when there are protected tokens', () => { + let token beforeEach(async () => { - await agent.setDesignatedSigner(randomAccount, { from: signerDesignator }) + token = await TokenMock.new(agent.address, 10000) + await agent.addProtectedToken(token.address, { from: authorized }) }) - it('isValidSignature returns false to an empty signature', async () => { - const emptySig = '0x' - assertIsValidSignature(false, await agent.isValidSignature(HASH, emptySig)) + it('has correct protected tokens length', async () => { + assert.equal(await agent.getProtectedTokensLength(), 1) }) - it('isValidSignature returns false to an invalid mode signature', async () => { - const invalidSignature = SIGNATURE_MODES.Invalid - assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidSignature)) + it('can access protected token', async () => { + const protectedTokenAddress = await agent.protectedTokens(0) + assert.equal(token.address, protectedTokenAddress) }) - it('isValidSignature returns false to an unspecified mode signature', async () => { - const unspecifiedSignature = SIGNATURE_MODES.NMode - assertIsValidSignature(false, await agent.isValidSignature(HASH, unspecifiedSignature)) + it('cannot access non-existent protected tokens', async () => { + assertRevert(agent.protectedTokens(1)) }) + }) - it('isValidSignature returns true to an invalid signature iff the hash was presigned', async () => { - const invalidSignature = SIGNATURE_MODES.Invalid - assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidSignature)) + context('> when there are no protected tokens', () => { + it('has correct protected tokens length', async () => { + assert.equal(await agent.getProtectedTokensLength(), 0) + }) - // Now presign it - await agent.presignHash(HASH, { from: presigner }) - assertIsValidSignature(true, await agent.isValidSignature(HASH, invalidSignature)) + it('cannot access non-existent protected tokens', async () => { + assertRevert(agent.protectedTokens(0)) }) }) + }) + + context('> Signing messages', () => { + const [_, nobody, presigner, signerDesignator] = accounts + const HASH = web3.sha3('hash') // careful as it may encode the data in the same way as solidity before hashing + + const SIGNATURE_MODES = { + Invalid: '0x00', + EIP712: '0x01', + EthSign: '0x02', + ERC1271: '0x03', + NMode: '0x04', + } + + const ERC1271_RETURN_VALID_SIGNATURE = '0x20c13b0b' + const ERC1271_RETURN_INVALID_SIGNATURE = '0x00000000' + + const assertIsValidSignature = (isValid, erc1271Return) => { + const expectedReturn = + isValid + ? ERC1271_RETURN_VALID_SIGNATURE + : ERC1271_RETURN_INVALID_SIGNATURE + + assert.equal(erc1271Return, expectedReturn, `Expected signature to be ${isValid ? '' : 'in'}valid (returned ${erc1271Return})`) + } + + beforeEach(async () => { + await acl.createPermission(presigner, agent.address, ADD_PRESIGNED_HASH_ROLE, root, { from: root }) + await acl.createPermission(signerDesignator, agent.address, DESIGNATE_SIGNER_ROLE, root, { from: root }) + }) + + it('complies with ERC165', async () => { + assert.isTrue(await agent.supportsInterface(ERC165_SUPPORT_INTERFACE_ID)) + assert.isFalse(await agent.supportsInterface(ERC165_SUPPORT_INVALID_ID)) + }) + + it('supports ERC1271 interface', async () => { + assert.isTrue(await agent.supportsInterface(ERC1271_INTERFACE_ID)) + }) + + it('doesn\'t support any other interface', async () => { + assert.isFalse(await agent.supportsInterface('0x12345678')) + assert.isFalse(await agent.supportsInterface('0x')) + }) + + it('isValidSignature returns false if there is not designated signer and hash isn\'t presigned', async () => { + assertIsValidSignature(false, await agent.isValidSignature(HASH, NO_SIG)) + }) - context('> Signature mode: self', () => { - it('cannot set itself as the designated signer', async () => { - await assertRevert(agent.setDesignatedSigner(agent.address, { from: signerDesignator }), errors.AGENT_DESIGNATED_TO_SELF) + it('presigns a hash', async () => { + await agent.presignHash(HASH, { from: presigner }) + + assertIsValidSignature(true, await agent.isValidSignature(HASH, NO_SIG)) + }) + + it('fails to presign a hash if not authorized', async () => { + await assertRevert(agent.presignHash(HASH, { from: nobody }), errors.APP_AUTH_FAILED) + assertIsValidSignature(false, await agent.isValidSignature(HASH, NO_SIG)) + }) + + context('> Designated signer', () => { + const ethSign = async (hash, signer) => { + const packedSig = await web3Sign(signer, hash) + + return { + r: ethUtil.toBuffer('0x' + packedSig.substring(2, 66)), + s: ethUtil.toBuffer('0x' + packedSig.substring(66, 130)), + v: parseInt(packedSig.substring(130, 132), 16) + 27, + mode: ethUtil.toBuffer(SIGNATURE_MODES.EthSign) + } + } + + const eip712Sign = async (hash, key) => ({ + mode: ethUtil.toBuffer(SIGNATURE_MODES.EIP712), + ...ethUtil.ecsign( + Buffer.from(hash.slice(2), 'hex'), + Buffer.from(key, 'hex') + ) + }) + + const signFunctionGenerator = (signFunction, signatureModifier) => ( + async (hash, signerOrKey, useLegacySig = false, useInvalidV = false) => { + const sig = await signFunction(hash, signerOrKey) + const v = + useInvalidV + ? ethUtil.toBuffer(2) // force set an invalid v + : ethUtil.toBuffer(sig.v - (useLegacySig ? 0 : 27)) + + const signature = '0x' + Buffer.concat([sig.mode, sig.r, sig.s, v]).toString('hex') + return signatureModifier(signature) + } + ) + + const addERC1271ModePrefix = (signature) => + `${SIGNATURE_MODES.ERC1271}${signature.slice(2)}` + + const createChildAgentGenerator = (designatedSigner) => + async () => { + const agentReceipt = await dao.newAppInstance(agentAppId, agentBase.address, '0x', false) + const childAgent = AgentLike.at(getNewProxyAddress(agentReceipt)) + + await childAgent.initialize() + await acl.createPermission(signerDesignator, childAgent.address, DESIGNATE_SIGNER_ROLE, root, { from: root }) + await childAgent.setDesignatedSigner(designatedSigner, { from: signerDesignator }) + + return childAgent.address + } + + const directSignatureTests = [ + { + name: 'EIP712', + signFunction: eip712Sign, + getSigner: () => '0x93070b307c373D7f9344859E909e3EEeF6E4Fd5a', + signerOrKey: '11bc31e7fef59610dfd6f95d2f78d2396c7b5477e4a9a54d72d9c1b76930e5c1', + notSignerOrKey: '7224b5bc510e01f75b10e3b6d6c903861ca91adb95a26406d1603e2d28a29e7f', + }, + { + name: 'EthSign', + signFunction: ethSign, + getSigner: () => accounts[7], + signerOrKey: accounts[7], + notSignerOrKey: accounts[8] + }, + ] + + const wrappedSignatureTests = directSignatureTests.map(signatureTest => ({ + ...signatureTest, + name: `ERC1271 -> ${signatureTest.name}`, + signatureModifier: addERC1271ModePrefix, + getSigner: createChildAgentGenerator(signatureTest.getSigner()), + })) + + const signatureTests = directSignatureTests.concat(wrappedSignatureTests) + + for (const { + name, + signFunction, + getSigner, + signerOrKey, + notSignerOrKey, + signatureModifier = sig => sig // defaults to identity function (returns input) + } of signatureTests) { + const sign = signFunctionGenerator(signFunction, signatureModifier) + + context(`> Signature mode: ${name}`, () => { + beforeEach(async () => { + const signer = await getSigner() + await agent.setDesignatedSigner(signer, { from: signerDesignator }) + }) + + it('isValidSignature returns true to a valid signature', async () => { + const signature = await sign(HASH, signerOrKey) + assertIsValidSignature(true, await agent.isValidSignature(HASH, signature)) + }) + + it('isValidSignature returns true to a valid signature with legacy version', async () => { + const legacyVersionSignature = await sign(HASH, signerOrKey, true) + assertIsValidSignature(true, await agent.isValidSignature(HASH, legacyVersionSignature)) + }) + + it('isValidSignature returns false to an invalid signature', async () => { + const badSignature = (await sign(HASH, signerOrKey)).slice(0, -2) // drop last byte + assertIsValidSignature(false, await agent.isValidSignature(HASH, badSignature)) + }) + + it('isValidSignature returns false to a signature with an invalid v', async () => { + const invalidVersionSignature = await sign(HASH, signerOrKey, false, true) + assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidVersionSignature)) + }) + + it('isValidSignature returns false to an unauthorized signer', async () => { + const otherSignature = await sign(HASH, notSignerOrKey) + assertIsValidSignature(false, await agent.isValidSignature(HASH, otherSignature)) + }) + }) + } + + context('> Signature mode: ERC1271', () => { + const ERC1271_SIG = SIGNATURE_MODES.ERC1271 + + const setDesignatedSignerContract = async (...params) => { + const designatedSigner = await DesignatedSigner.new(...params) + return agent.setDesignatedSigner(designatedSigner.address, { from: signerDesignator }) + } + + it('isValidSignature returns true if designated signer returns true', async () => { + // true - ERC165 interface compliant + // true - any sigs are valid + // false - doesn't revert on checking sig + // false - doesn't modify state on checking sig + await setDesignatedSignerContract(true, true, false, false) + + assertIsValidSignature(true, await agent.isValidSignature(HASH, ERC1271_SIG)) + }) + + it('isValidSignature returns false if designated signer returns false', async () => { + // true - ERC165 interface compliant + // false - sigs are invalid + // false - doesn't revert on checking sig + // false - doesn't modify state on checking sig + await setDesignatedSignerContract(true, false, false, false) + + // Signature fails check + assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG)) + }) + + it('isValidSignature returns true even if the designated signer doesnt support the interface', async () => { + // false - not ERC165 interface compliant + // true - any sigs are valid + // false - doesn't revert on checking sig + // false - doesn't modify state on checking sig + await setDesignatedSignerContract(false, true, false, false) + + assertIsValidSignature(true, await agent.isValidSignature(HASH, ERC1271_SIG)) + }) + + it('isValidSignature returns false if designated signer reverts', async () => { + // true - ERC165 interface compliant + // true - any sigs are valid + // true - reverts on checking sig + // false - doesn't modify state on checking sig + await setDesignatedSignerContract(true, true, true, false) + + // Reverts on checking + assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG)) + }) + + it('isValidSignature returns false if designated signer attempts to modify state', async () => { + // true - ERC165 interface compliant + // true - any sigs are valid + // false - doesn't revert on checking sig + // true - modifies state on checking sig + await setDesignatedSignerContract(true, true, false, true) + + // Checking costs too much gas + assertIsValidSignature(false, await agent.isValidSignature(HASH, ERC1271_SIG)) + }) + }) + + context(`> Signature mode: invalid modes`, () => { + const randomAccount = accounts[9] + + beforeEach(async () => { + await agent.setDesignatedSigner(randomAccount, { from: signerDesignator }) + }) + + it('isValidSignature returns false to an empty signature', async () => { + const emptySig = '0x' + assertIsValidSignature(false, await agent.isValidSignature(HASH, emptySig)) + }) + + it('isValidSignature returns false to an invalid mode signature', async () => { + const invalidSignature = SIGNATURE_MODES.Invalid + assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidSignature)) + }) + + it('isValidSignature returns false to an unspecified mode signature', async () => { + const unspecifiedSignature = SIGNATURE_MODES.NMode + assertIsValidSignature(false, await agent.isValidSignature(HASH, unspecifiedSignature)) + }) + + it('isValidSignature returns true to an invalid signature iff the hash was presigned', async () => { + const invalidSignature = SIGNATURE_MODES.Invalid + assertIsValidSignature(false, await agent.isValidSignature(HASH, invalidSignature)) + + // Now presign it + await agent.presignHash(HASH, { from: presigner }) + assertIsValidSignature(true, await agent.isValidSignature(HASH, invalidSignature)) + }) + }) + + context('> Signature mode: self', () => { + it('cannot set itself as the designated signer', async () => { + await assertRevert(agent.setDesignatedSigner(agent.address, { from: signerDesignator }), errors.AGENT_DESIGNATED_TO_SELF) + }) }) }) })