From aecd344d58830c3a7b0d877604c18181122d7f06 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Wed, 8 Nov 2017 17:36:15 -0200 Subject: [PATCH 01/34] fixed heritance --- contracts/MultiSigTokenWallet.sol | 350 +-------------------------- contracts/MultiSigWallet.sol | 378 ++++++++++++++++++++++++++++++ 2 files changed, 380 insertions(+), 348 deletions(-) create mode 100644 contracts/MultiSigWallet.sol diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index d1f12f9..bd1fedc 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -1,114 +1,21 @@ pragma solidity ^0.4.15; +import "./MultiSigWallet.sol"; import "./ERC20.sol"; -contract MultiSigTokenWallet { +contract MultiSigTokenWallet is MultiSigWallet { - address[] public owners; address[] public tokens; - mapping (uint => Transaction) public transactions; - mapping (uint => mapping (address => bool)) public confirmations; - uint public transactionCount; - mapping (address => uint) public tokenBalances; - mapping (address => bool) public isOwner; mapping (address => address[]) public userList; - uint public required; uint public nonce; - struct Transaction { - address destination; - uint value; - bytes data; - bool executed; - } - - uint constant public MAX_OWNER_COUNT = 50; - - event Confirmation(address indexed _sender, uint indexed _transactionId); - event Revocation(address indexed _sender, uint indexed _transactionId); - event Submission(uint indexed _transactionId); - event Execution(uint indexed _transactionId); - event ExecutionFailure(uint indexed _transactionId); - event Deposit(address indexed _sender, uint _value); event TokenDeposit(address _token, address indexed _sender, uint _value); - event OwnerAddition(address indexed _owner); - event OwnerRemoval(address indexed _owner); - event RequirementChange(uint _required); - - modifier onlyWallet() { - require (msg.sender == address(this)); - _; - } - - modifier ownerDoesNotExist(address owner) { - require (!isOwner[owner]); - _; - } - - modifier ownerExists(address owner) { - require (isOwner[owner]); - _; - } - - modifier transactionExists(uint transactionId) { - require (transactions[transactionId].destination != 0); - _; - } - - modifier confirmed(uint transactionId, address owner) { - require (confirmations[transactionId][owner]); - _; - } - - modifier notConfirmed(uint transactionId, address owner) { - require(!confirmations[transactionId][owner]); - _; - } - - modifier notExecuted(uint transactionId) { - require (!transactions[transactionId].executed); - _; - } - - modifier notNull(address _address) { - require (_address != 0); - _; - } - - modifier validRequirement(uint ownerCount, uint _required) { - require (ownerCount <= MAX_OWNER_COUNT && _required <= ownerCount && _required != 0 && ownerCount != 0); - _; - } - - /// @dev Fallback function allows to deposit ether. - function() - payable - { - if (msg.value > 0) - Deposit(msg.sender, msg.value); - } /** * Public functions * **/ - /// @dev Contract constructor sets initial owners and required number of confirmations. - /// @param _owners List of initial owners. - /// @param _required Number of required confirmations. - function constructor(address[] _owners, uint _required) - public - validRequirement(_owners.length, _required) - { - require(owners.length == 0 && required == 0); - for (uint i = 0; i < _owners.length; i++) { - require(!isOwner[_owners[i]] && _owners[i] != 0); - isOwner[_owners[i]] = true; - } - owners = _owners; - required = _required; - } - /** * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. * @param _token the token contract address @@ -188,63 +95,6 @@ contract MultiSigTokenWallet { deposit(_from, _amount, _token, _data); } - - /// @dev Allows to add a new owner. Transaction has to be sent by wallet. - /// @param owner Address of new owner. - function addOwner(address owner) - public - onlyWallet - ownerDoesNotExist(owner) - notNull(owner) - validRequirement(owners.length + 1, required) - { - isOwner[owner] = true; - owners.push(owner); - OwnerAddition(owner); - } - - /// @dev Allows to remove an owner. Transaction has to be sent by wallet. - /// @param owner Address of owner. - function removeOwner(address owner) - public - onlyWallet - ownerExists(owner) - { - isOwner[owner] = false; - uint _len = owners.length - 1; - for (uint i = 0; i < _len; i++) { - if (owners[i] == owner) { - owners[i] = owners[owners.length - 1]; - break; - } - } - owners.length -= 1; - if (required > owners.length) - changeRequirement(owners.length); - OwnerRemoval(owner); - } - - /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet. - /// @param owner Address of owner to be replaced. - /// @param owner Address of new owner. - function replaceOwner(address owner, address newOwner) - public - onlyWallet - ownerExists(owner) - ownerDoesNotExist(newOwner) - { - for (uint i = 0; i < owners.length; i++) { - if (owners[i] == owner) { - owners[i] = newOwner; - break; - } - } - isOwner[owner] = false; - isOwner[newOwner] = true; - OwnerRemoval(owner); - OwnerAddition(newOwner); - } - /** * @dev gives full ownership of this wallet to `_dest` removing older owners from wallet * @param _dest the address of new controller @@ -263,73 +113,6 @@ contract MultiSigTokenWallet { } } - /// @dev Allows to change the number of required confirmations. Transaction has to be sent by wallet. - /// @param _required Number of required confirmations. - function changeRequirement(uint _required) - public - onlyWallet - validRequirement(owners.length, _required) - { - required = _required; - RequirementChange(_required); - } - - /// @dev Allows an owner to submit and confirm a transaction. - /// @param destination Transaction target address. - /// @param value Transaction ether value. - /// @param data Transaction data payload. - /// @return Returns transaction ID. - function submitTransaction(address destination, uint value, bytes data) - public - returns (uint transactionId) - { - transactionId = addTransaction(destination, value, data); - confirmTransaction(transactionId); - } - - /// @dev Allows an owner to confirm a transaction. - /// @param transactionId Transaction ID. - function confirmTransaction(uint transactionId) - public - ownerExists(msg.sender) - transactionExists(transactionId) - notConfirmed(transactionId, msg.sender) - { - confirmations[transactionId][msg.sender] = true; - Confirmation(msg.sender, transactionId); - executeTransaction(transactionId); - } - - /// @dev Allows an owner to revoke a confirmation for a transaction. - /// @param transactionId Transaction ID. - function revokeConfirmation(uint transactionId) - public - ownerExists(msg.sender) - confirmed(transactionId, msg.sender) - notExecuted(transactionId) - { - confirmations[transactionId][msg.sender] = false; - Revocation(msg.sender, transactionId); - } - - /// @dev Allows anyone to execute a confirmed transaction. - /// @param transactionId Transaction ID. - function executeTransaction(uint transactionId) - public - notExecuted(transactionId) - { - if (isConfirmed(transactionId)) { - Transaction storage txx = transactions[transactionId]; - txx.executed = true; - if (txx.destination.call.value(txx.value)(txx.data)) { - Execution(transactionId); - } else { - ExecutionFailure(transactionId); - txx.executed = false; - } - } - } - /** * @dev withdraw all recognized tokens balances and ether to `_dest` * @param _dest the address of receiver @@ -387,47 +170,6 @@ contract MultiSigTokenWallet { bool result = ERC20(_tokenAddr).transfer(_dest, _amount); assert(result); } - - /// @dev Returns the confirmation status of a transaction. - /// @param transactionId Transaction ID. - /// @return Confirmation status. - function isConfirmed(uint transactionId) - public - constant - returns (bool) - { - uint count = 0; - for (uint i = 0; i < owners.length; i++) { - if (confirmations[transactionId][owners[i]]) - count += 1; - if (count == required) - return true; - } - } - - /* - * Internal functions - */ - /// @dev Adds a new transaction to the transaction mapping, if transaction does not exist yet. - /// @param destination Transaction target address. - /// @param value Transaction ether value. - /// @param data Transaction data payload. - /// @return Returns transaction ID. - function addTransaction(address destination, uint value, bytes data) - internal - notNull(destination) - returns (uint transactionId) - { - transactionId = transactionCount; - transactions[transactionId] = Transaction({ - destination: destination, - value: value, - data: data, - executed: false - }); - transactionCount += 1; - Submission(transactionId); - } /** * @dev register the deposit @@ -448,45 +190,6 @@ contract MultiSigTokenWallet { /* * Web3 call functions */ - /// @dev Returns number of confirmations of a transaction. - /// @param transactionId Transaction ID. - /// @return Number of confirmations. - function getConfirmationCount(uint transactionId) - public - constant - returns (uint count) - { - for (uint i = 0; i < owners.length; i++) { - if (confirmations[transactionId][owners[i]]) - count += 1; - } - } - - /// @dev Returns total number of transactions after filters are applied. - /// @param pending Include pending transactions. - /// @param executed Include executed transactions. - /// @return Total number of transactions after filters are applied. - function getTransactionCount(bool pending, bool executed) - public - constant - returns (uint count) - { - for (uint i = 0; i < transactionCount; i++) { - if (pending && !transactions[i].executed || executed && transactions[i].executed) - count += 1; - } - } - - /// @dev Returns list of owners. - /// @return List of owner addresses. - function getOwners() - public - constant - returns (address[]) - { - return owners; - } - /// @dev Returns list of tokens. /// @return List of token addresses. function getTokenList() @@ -497,53 +200,4 @@ contract MultiSigTokenWallet { return tokens; } - /// @dev Returns array with owner addresses, which confirmed transaction. - /// @param transactionId Transaction ID. - /// @return Returns array of owner addresses. - function getConfirmations(uint transactionId) - public - constant - returns (address[] _confirmations) - { - address[] memory confirmationsTemp = new address[](owners.length); - uint count = 0; - uint i; - for (i = 0; i < owners.length; i++) { - if (confirmations[transactionId][owners[i]]) { - confirmationsTemp[count] = owners[i]; - count += 1; - } - } - _confirmations = new address[](count); - for (i = 0; i < count; i++) { - _confirmations[i] = confirmationsTemp[i]; - } - } - - /// @dev Returns list of transaction IDs in defined range. - /// @param from Index start position of transaction array. - /// @param to Index end position of transaction array. - /// @param pending Include pending transactions. - /// @param executed Include executed transactions. - /// @return Returns array of transaction IDs. - function getTransactionIds(uint from, uint to, bool pending, bool executed) - public - constant - returns (uint[] _transactionIds) - { - uint[] memory transactionIdsTemp = new uint[](transactionCount); - uint count = 0; - uint i; - for (i = 0; i < transactionCount; i++) { - if (pending && !transactions[i].executed || executed && transactions[i].executed) { - transactionIdsTemp[count] = i; - count += 1; - } - } - _transactionIds = new uint[](to - from); - for (i = from; i < to; i++) { - _transactionIds[i - from] = transactionIdsTemp[i]; - } - } - } diff --git a/contracts/MultiSigWallet.sol b/contracts/MultiSigWallet.sol new file mode 100644 index 0000000..642aef3 --- /dev/null +++ b/contracts/MultiSigWallet.sol @@ -0,0 +1,378 @@ +pragma solidity ^0.4.17; + + +/// @title Multisignature wallet - Allows multiple parties to agree on transactions before execution. +/// @author Stefan George - +contract MultiSigWallet { + + /* + * Events + */ + event Confirmation(address indexed sender, uint indexed transactionId); + event Revocation(address indexed sender, uint indexed transactionId); + event Submission(uint indexed transactionId); + event Execution(uint indexed transactionId); + event ExecutionFailure(uint indexed transactionId); + event Deposit(address indexed sender, uint value); + event OwnerAddition(address indexed owner); + event OwnerRemoval(address indexed owner); + event RequirementChange(uint required); + + /* + * Constants + */ + uint constant public MAX_OWNER_COUNT = 50; + + /* + * Storage + */ + mapping (uint => Transaction) public transactions; + mapping (uint => mapping (address => bool)) public confirmations; + mapping (address => bool) public isOwner; + address[] public owners; + uint public required; + uint public transactionCount; + + struct Transaction { + address destination; + uint value; + bytes data; + bool executed; + } + + /* + * Modifiers + */ + modifier onlyWallet() { + require (msg.sender == address(this)); + _; + } + + modifier ownerDoesNotExist(address owner) { + require (!isOwner[owner]); + _; + } + + modifier ownerExists(address owner) { + require (isOwner[owner]); + _; + } + + modifier transactionExists(uint transactionId) { + require (transactions[transactionId].destination != 0); + _; + } + + modifier confirmed(uint transactionId, address owner) { + require (confirmations[transactionId][owner]); + _; + } + + modifier notConfirmed(uint transactionId, address owner) { + require(!confirmations[transactionId][owner]); + _; + } + + modifier notExecuted(uint transactionId) { + require (!transactions[transactionId].executed); + _; + } + + modifier notNull(address _address) { + require (_address != 0); + _; + } + + modifier validRequirement(uint ownerCount, uint _required) { + require (ownerCount <= MAX_OWNER_COUNT && _required <= ownerCount && _required != 0 && ownerCount != 0); + _; + } + + /// @dev Fallback function allows to deposit ether. + function () + public + payable + { + if (msg.value > 0) + Deposit(msg.sender, msg.value); + } + + /* + * Public functions + */ + /// @dev Constructor calls function `constructor(address[],uint256)` + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + function MultiSigWallet(address[] _owners, uint _required) + public + { + constructor(_owners, _required); + } + + /// @dev Contract constructor sets initial owners and required number of confirmations, can only be called once. + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + function constructor(address[] _owners, uint _required) + public + validRequirement(_owners.length, _required) + { + require(owners.length == 0 && required == 0); + for (uint i = 0; i < _owners.length; i++) { + require(!isOwner[_owners[i]] && _owners[i] != 0); + isOwner[_owners[i]] = true; + } + owners = _owners; + required = _required; + } + + /// @dev Allows to add a new owner. Transaction has to be sent by wallet. + /// @param owner Address of new owner. + function addOwner(address owner) + public + onlyWallet + ownerDoesNotExist(owner) + notNull(owner) + validRequirement(owners.length + 1, required) + { + isOwner[owner] = true; + owners.push(owner); + OwnerAddition(owner); + } + + /// @dev Allows to remove an owner. Transaction has to be sent by wallet. + /// @param owner Address of owner. + function removeOwner(address owner) + public + onlyWallet + ownerExists(owner) + { + isOwner[owner] = false; + for (uint i=0; i owners.length) + changeRequirement(owners.length); + OwnerRemoval(owner); + } + + /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet. + /// @param owner Address of owner to be replaced. + /// @param newOwner Address of new owner. + function replaceOwner(address owner, address newOwner) + public + onlyWallet + ownerExists(owner) + ownerDoesNotExist(newOwner) + { + for (uint i=0; i Date: Wed, 8 Nov 2017 17:53:38 -0200 Subject: [PATCH 02/34] remove internal balances, better handle bad tokens, better handle race conditions --- contracts/MultiSigTokenWallet.sol | 149 +++++++++++++++--------------- 1 file changed, 74 insertions(+), 75 deletions(-) diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index bd1fedc..8490f42 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -6,11 +6,11 @@ import "./ERC20.sol"; contract MultiSigTokenWallet is MultiSigWallet { address[] public tokens; - mapping (address => uint) public tokenBalances; + mapping (address => uint) watchedPos; mapping (address => address[]) public userList; uint public nonce; - event TokenDeposit(address _token, address indexed _sender, uint _value); + event TokenDeposit(address indexed token, address indexed sender, uint value); /** * Public functions @@ -47,41 +47,59 @@ contract MultiSigTokenWallet is MultiSigWallet { _deposited(_from, _amount, _token, _data); } } + /** * @notice watches for balance in a token contract * @param _tokenAddr the token contract address **/ function watch(address _tokenAddr) + public ownerExists(msg.sender) { - uint oldBal = tokenBalances[_tokenAddr]; - uint newBal = ERC20(_tokenAddr).balanceOf(this); - if (newBal > oldBal) { - _deposited(0x0, newBal-oldBal, _tokenAddr, new bytes(0)); - } + require(watchedPos[_tokenAddr] == 0); + require(ERC20(_tokenAddr).balanceOf(address(this)) > 0); + tokens.push(_tokenAddr); + watchedPos[_tokenAddr] = tokens.length; } + /** + * @notice watches for balance in a token contract + * @param _tokenAddr the token contract address + **/ + function unwatch(address _tokenAddr) + public + ownerExists(msg.sender) + { + require(watchedPos[_tokenAddr] > 0); + tokens[watchedPos[_tokenAddr] - 1] = tokens[tokens.length - 1]; + delete watchedPos[_tokenAddr]; + tokens.length--; + } + + /** + * @notice set token list that will be used at `withdrawEverything(msg.sender)` and `withdrawAllTokens(msg.sender)` function when sending to `msg.sender`. + **/ function setMyTokenList(address[] _tokenList) public { userList[msg.sender] = _tokenList; } - function setTokenList(address[] _tokenList) - onlyWallet - { - tokens = _tokenList; - } - /** * @notice ERC23 Token fallback * @param _from address incoming token * @param _amount incoming amount **/ - function tokenFallback(address _from, uint _amount, bytes _data) + function tokenFallback( + address _from, + uint _amount, + bytes _data + ) public + returns (bool) { _deposited(_from, _amount, msg.sender, _data); + return true; } /** @@ -91,49 +109,25 @@ contract MultiSigTokenWallet is MultiSigWallet { * @param _token the token contract address * @param _data (might be used by child classes) */ - function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) { + function receiveApproval( + address _from, + uint256 _amount, + address _token, + bytes _data + ) + public + { deposit(_from, _amount, _token, _data); } /** - * @dev gives full ownership of this wallet to `_dest` removing older owners from wallet - * @param _dest the address of new controller - **/ - function releaseWallet(address _dest) - public - notNull(_dest) - ownerDoesNotExist(_dest) - onlyWallet - { - address[] memory _owners = owners; - uint numOwners = _owners.length; - addOwner(_dest); - for (uint i = 0; i < numOwners; i++) { - removeOwner(_owners[i]); - } - } - - /** - * @dev withdraw all recognized tokens balances and ether to `_dest` + * @dev withdraw all watched tokens balances and ether to `_dest` * @param _dest the address of receiver **/ function withdrawEverything(address _dest) public notNull(_dest) onlyWallet - { - withdrawAllTokens(_dest); - _dest.transfer(this.balance); - } - - /** - * @dev withdraw all recognized tokens balances to `_dest` - * @param _dest the address of receiver - **/ - function withdrawAllTokens(address _dest) - public - notNull(_dest) - onlyWallet { address[] memory _tokenList; if (userList[_dest].length > 0) { @@ -141,49 +135,54 @@ contract MultiSigTokenWallet is MultiSigWallet { } else { _tokenList = tokens; } + withdrawAllTokens(_dest, _tokenList); + _dest.transfer(this.balance); + } + + /** + * @dev withdraw all tokens in list and ether to `_dest` + * @param _dest the address of receiver + * @param _tokenList the list of tokens to withdraw all balance + **/ + function withdrawEverything(address _dest, address[] _tokenList) + public + notNull(_dest) + onlyWallet + { + withdrawAllTokens(_dest, _tokenList); + _dest.transfer(this.balance); + } + + /** + * @dev withdraw all listed tokens balances to `_dest` + * @param _dest the address of receiver + * @param _tokenList the list of tokens to withdraw all balance + **/ + function withdrawAllTokens(address _dest, address[] _tokenList) + public + notNull(_dest) + onlyWallet + { uint len = _tokenList.length; for (uint i = 0;i < len; i++) { address _tokenAddr = _tokenList[i]; - uint _amount = tokenBalances[_tokenAddr]; + uint _amount = ERC20(_tokenAddr).balanceOf(address(this)); if (_amount > 0) { - delete tokenBalances[_tokenAddr]; ERC20(_tokenAddr).transfer(_dest, _amount); } } } - /** - * @dev withdraw `_tokenAddr` `_amount` to `_dest` - * @param _tokenAddr the address of the token - * @param _dest the address of receiver - * @param _amount the number of tokens to send - **/ - function withdrawToken(address _tokenAddr, address _dest, uint _amount) - public - notNull(_dest) - onlyWallet - { - require(_amount > 0); - uint _balance = tokenBalances[_tokenAddr]; - require(_amount <= _balance); - tokenBalances[_tokenAddr] = _balance - _amount; - bool result = ERC20(_tokenAddr).transfer(_dest, _amount); - assert(result); - } - /** * @dev register the deposit **/ function _deposited(address _from, uint _amount, address _tokenAddr, bytes) internal { - TokenDeposit(_tokenAddr,_from,_amount); - nonce++; - if (tokenBalances[_tokenAddr] == 0) { + TokenDeposit(_tokenAddr, _from, _amount); + if (watchedPos[_tokenAddr] > 0) { tokens.push(_tokenAddr); - tokenBalances[_tokenAddr] = ERC20(_tokenAddr).balanceOf(this); - } else { - tokenBalances[_tokenAddr] += _amount; + watchedPos[_tokenAddr] = tokens.length; } } @@ -200,4 +199,4 @@ contract MultiSigTokenWallet is MultiSigWallet { return tokens; } -} +} \ No newline at end of file From 54fcbed51b879e0e3803072a15799b8494b7c9b9 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Wed, 8 Nov 2017 18:18:07 -0200 Subject: [PATCH 03/34] remove nonce and assert that withdrawAllTokens will not fail because bad tokens --- contracts/MultiSigTokenWallet.sol | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index 8490f42..c017ac2 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -8,7 +8,6 @@ contract MultiSigTokenWallet is MultiSigWallet { address[] public tokens; mapping (address => uint) watchedPos; mapping (address => address[]) public userList; - uint public nonce; event TokenDeposit(address indexed token, address indexed sender, uint value); @@ -39,13 +38,10 @@ contract MultiSigTokenWallet is MultiSigWallet { { if (_from == address(this)) return; - uint _nonce = nonce; bool result = ERC20(_token).transferFrom(_from, this, _amount); - assert(result); - //ERC23 not executed _deposited tokenFallback by - if (nonce == _nonce) { - _deposited(_from, _amount, _token, _data); - } + require(result); + _deposited(_from, _amount, _token, _data); + } /** @@ -168,7 +164,7 @@ contract MultiSigTokenWallet is MultiSigWallet { address _tokenAddr = _tokenList[i]; uint _amount = ERC20(_tokenAddr).balanceOf(address(this)); if (_amount > 0) { - ERC20(_tokenAddr).transfer(_dest, _amount); + ERC20(_tokenAddr).call(bytes4(keccak256("transfer(address,uint256)")), _dest, _amount); } } } From 53cc9c2c0e776c185b7a6b692f2629a95aeef02c Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 06:21:06 -0200 Subject: [PATCH 04/34] remove token watch related stuff --- contracts/MultiSigTokenWallet.sol | 89 +------------------------------ 1 file changed, 2 insertions(+), 87 deletions(-) diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index c017ac2..ebec928 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -5,10 +5,6 @@ import "./ERC20.sol"; contract MultiSigTokenWallet is MultiSigWallet { - address[] public tokens; - mapping (address => uint) watchedPos; - mapping (address => address[]) public userList; - event TokenDeposit(address indexed token, address indexed sender, uint value); /** @@ -40,47 +36,10 @@ contract MultiSigTokenWallet is MultiSigWallet { return; bool result = ERC20(_token).transferFrom(_from, this, _amount); require(result); - _deposited(_from, _amount, _token, _data); + TokenDeposit(_token, _from, _amount); } - /** - * @notice watches for balance in a token contract - * @param _tokenAddr the token contract address - **/ - function watch(address _tokenAddr) - public - ownerExists(msg.sender) - { - require(watchedPos[_tokenAddr] == 0); - require(ERC20(_tokenAddr).balanceOf(address(this)) > 0); - tokens.push(_tokenAddr); - watchedPos[_tokenAddr] = tokens.length; - } - - /** - * @notice watches for balance in a token contract - * @param _tokenAddr the token contract address - **/ - function unwatch(address _tokenAddr) - public - ownerExists(msg.sender) - { - require(watchedPos[_tokenAddr] > 0); - tokens[watchedPos[_tokenAddr] - 1] = tokens[tokens.length - 1]; - delete watchedPos[_tokenAddr]; - tokens.length--; - } - - /** - * @notice set token list that will be used at `withdrawEverything(msg.sender)` and `withdrawAllTokens(msg.sender)` function when sending to `msg.sender`. - **/ - function setMyTokenList(address[] _tokenList) - public - { - userList[msg.sender] = _tokenList; - } - /** * @notice ERC23 Token fallback * @param _from address incoming token @@ -94,7 +53,7 @@ contract MultiSigTokenWallet is MultiSigWallet { public returns (bool) { - _deposited(_from, _amount, msg.sender, _data); + TokenDeposit(msg.sender, _from, _amount); return true; } @@ -116,24 +75,6 @@ contract MultiSigTokenWallet is MultiSigWallet { deposit(_from, _amount, _token, _data); } - /** - * @dev withdraw all watched tokens balances and ether to `_dest` - * @param _dest the address of receiver - **/ - function withdrawEverything(address _dest) - public - notNull(_dest) - onlyWallet - { - address[] memory _tokenList; - if (userList[_dest].length > 0) { - _tokenList = userList[_dest]; - } else { - _tokenList = tokens; - } - withdrawAllTokens(_dest, _tokenList); - _dest.transfer(this.balance); - } /** * @dev withdraw all tokens in list and ether to `_dest` @@ -169,30 +110,4 @@ contract MultiSigTokenWallet is MultiSigWallet { } } - /** - * @dev register the deposit - **/ - function _deposited(address _from, uint _amount, address _tokenAddr, bytes) - internal - { - TokenDeposit(_tokenAddr, _from, _amount); - if (watchedPos[_tokenAddr] > 0) { - tokens.push(_tokenAddr); - watchedPos[_tokenAddr] = tokens.length; - } - } - - /* - * Web3 call functions - */ - /// @dev Returns list of tokens. - /// @return List of token addresses. - function getTokenList() - public - constant - returns (address[]) - { - return tokens; - } - } \ No newline at end of file From 1b2a941869d7e722146f3d3f1fc12f9981e42189 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 06:21:31 -0200 Subject: [PATCH 05/34] abstract the DelegatedCall logic --- contracts/DelegatedCall.sol | 71 +++++++++++++++++++++++++++++++++++++ contracts/MultiSigStub.sol | 56 +++-------------------------- 2 files changed, 76 insertions(+), 51 deletions(-) create mode 100644 contracts/DelegatedCall.sol diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol new file mode 100644 index 0000000..9155daf --- /dev/null +++ b/contracts/DelegatedCall.sol @@ -0,0 +1,71 @@ +pragma solidity ^0.4.12; + +/** + * @title DelegatedCall + * @author Ricardo Guilherme Schmidt (Status Research & Development GmbH) + * Abstract contract that delegates all calls to contract returned by abstract function `_getDelegatedContract` + */ +contract DelegatedCall { + + /** + * @dev delegates the call of this function + */ + modifier delegated + { + uint inSize = msg.data.length; + bytes32 inDataPtr = _malloc(inSize); + + assembly { + calldatacopy(inDataPtr, 0x0, inSize) + } + + uint256 outSize; + bytes32 outDataPtr; + + _delegatecall(inDataPtr, inSize); + _; + assembly { + return(0, outSize) + } + } + + /** + * @dev defines the address for delegation of calls + */ + function _getDelegatedContract() + internal + returns(address); + + /** + * @dev allocates memory to a pointer + */ + function _malloc(uint size) + internal + returns(bytes32 ptr) + { + assembly { + ptr := mload(0x40) + mstore(0x40, add(ptr, size)) + } + } + + /** + * @dev delegates the data in pointer + */ + function _delegatecall(bytes32 inDataPtr, uint inSize) + internal + returns(bytes32 outDataPtr, uint256 outSize) + { + outSize = 0x20; + address target = _getDelegatedContract(); + outDataPtr = _malloc(outSize); + bool failed; + + assembly { + failed := iszero(delegatecall(sub(gas, 10000), target, inDataPtr, inSize, outDataPtr, outSize)) + } + + assert(!failed); + } + +} diff --git a/contracts/MultiSigStub.sol b/contracts/MultiSigStub.sol index 4857129..1234adb 100644 --- a/contracts/MultiSigStub.sol +++ b/contracts/MultiSigStub.sol @@ -1,13 +1,14 @@ pragma solidity ^0.4.15; +import "./DelegatedCall.sol"; + /** * @title MultiSigStub * Contract that delegates calls to a library to build a full MultiSigWallet that is cheap to create. */ -contract MultiSigStub { +contract MultiSigStub is DelegatedCall { address[] public owners; - address[] public tokens; mapping (uint => Transaction) public transactions; mapping (uint => mapping (address => bool)) public confirmations; uint public transactionCount; @@ -34,21 +35,6 @@ contract MultiSigStub { _delegatecall(mData, size); } - modifier delegated { - uint size = msg.data.length; - bytes32 mData = _malloc(size); - - assembly { - calldatacopy(mData, 0x0, size) - } - - bytes32 mResult = _delegatecall(mData, size); - _; - assembly { - return(mResult, 0x20) - } - } - function() payable delegated @@ -70,20 +56,7 @@ contract MultiSigStub { { } - - function watch(address _tokenAddr) - public - delegated - { - - } - - function setMyTokenList(address[] _tokenList) - public - delegated - { - } /// @dev Returns the confirmation status of a transaction. /// @param transactionId Transaction ID. /// @return Confirmation status. @@ -99,15 +72,6 @@ contract MultiSigStub { /* * Web3 call functions */ - function tokenBalances(address tokenAddress) - public - constant - delegated - returns (uint) - { - - } - /// @dev Returns number of confirmations of a transaction. /// @param transactionId Transaction ID. @@ -144,16 +108,6 @@ contract MultiSigStub { return owners; } - /// @dev Returns list of tokens. - /// @return List of token addresses. - function getTokenList() - public - constant - returns (address[]) - { - return tokens; - } - /// @dev Returns array with owner addresses, which confirmed transaction. /// @param transactionId Transaction ID. /// @return Returns array of owner addresses. @@ -206,6 +160,7 @@ contract MultiSigStub { function _malloc(uint size) private + pure returns(bytes32 mData) { assembly { @@ -218,8 +173,7 @@ contract MultiSigStub { private returns(bytes32 mResult) { - address target = 0xc0FFeEE61948d8993864a73a099c0E38D887d3F4; //Multinetwork - mResult = _malloc(32); + address target = 0xCaFFE810d0dF52E27DC580AD4a3C6283B0094291; bool failed; assembly { From 5fe2bb9b5298f8140ec10b501b870c31c63a9a00 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 06:26:05 -0200 Subject: [PATCH 06/34] explicitally set function as public --- contracts/MultiSigFactory.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/MultiSigFactory.sol b/contracts/MultiSigFactory.sol index 62bc00a..99e80ae 100644 --- a/contracts/MultiSigFactory.sol +++ b/contracts/MultiSigFactory.sol @@ -6,7 +6,10 @@ contract MultiSigFactory { event Create(address indexed caller, address createdContract); - function create(address[] owners, uint256 required) returns (address wallet) { + function create(address[] owners, uint256 required) + public + returns (address wallet) + { wallet = new MultiSigStub(owners, required); Create(msg.sender, wallet); } From 8d149a396b60d5c28475ba77d761cf71a0f32159 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 06:43:00 -0200 Subject: [PATCH 07/34] fix stub to use parent methods --- contracts/MultiSigStub.sol | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/contracts/MultiSigStub.sol b/contracts/MultiSigStub.sol index 1234adb..3bec475 100644 --- a/contracts/MultiSigStub.sol +++ b/contracts/MultiSigStub.sol @@ -157,30 +157,11 @@ contract MultiSigStub is DelegatedCall { } } - - function _malloc(uint size) - private - pure - returns(bytes32 mData) + function _getDelegatedContract() + internal + returns(address) { - assembly { - mData := mload(0x40) - mstore(0x40, add(mData, size)) - } - } - - function _delegatecall(bytes32 mData, uint size) - private - returns(bytes32 mResult) - { - address target = 0xCaFFE810d0dF52E27DC580AD4a3C6283B0094291; - bool failed; - - assembly { - failed := iszero(delegatecall(sub(gas, 10000), target, mData, size, mResult, 0x20)) - } - - assert(!failed); + return 0xCaFFE810d0dF52E27DC580AD4a3C6283B0094291; } } \ No newline at end of file From 451984b55c937706e183187728243ed7da288a66 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 08:17:08 -0200 Subject: [PATCH 08/34] fix fixed length returns at delegatedcall --- contracts/DelegatedCall.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol index 9155daf..9dc62ff 100644 --- a/contracts/DelegatedCall.sol +++ b/contracts/DelegatedCall.sol @@ -19,10 +19,10 @@ contract DelegatedCall { calldatacopy(inDataPtr, 0x0, inSize) } - uint256 outSize; bytes32 outDataPtr; + uint256 outSize; - _delegatecall(inDataPtr, inSize); + (outDataPtr, outSize) = _delegatecall(inDataPtr, inSize); _; assembly { return(0, outSize) From e43d1c5d78e6bb9b85b4abdfb66f335c521ef5cd Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 13:05:57 -0200 Subject: [PATCH 09/34] support dynamic data return --- contracts/DelegatedCall.sol | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol index 9dc62ff..18b2851 100644 --- a/contracts/DelegatedCall.sol +++ b/contracts/DelegatedCall.sol @@ -25,7 +25,7 @@ contract DelegatedCall { (outDataPtr, outSize) = _delegatecall(inDataPtr, inSize); _; assembly { - return(0, outSize) + return(outDataPtr, outSize) } } @@ -56,16 +56,17 @@ contract DelegatedCall { internal returns(bytes32 outDataPtr, uint256 outSize) { - outSize = 0x20; address target = _getDelegatedContract(); - outDataPtr = _malloc(outSize); bool failed; - assembly { - failed := iszero(delegatecall(sub(gas, 10000), target, inDataPtr, inSize, outDataPtr, outSize)) + failed := iszero(delegatecall(sub(gas, 10000), target, inDataPtr, inSize, 0, 0)) + outSize := returndatasize + } + require(!failed); + outDataPtr = _malloc(outSize); + assembly { + returndatacopy(outDataPtr, 0, outSize) } - - assert(!failed); } } From fd75fc1ba2ad52bf41ebd762a45db2dca0fa2b6b Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 13:06:24 -0200 Subject: [PATCH 10/34] Change to safecall into ERC20 --- contracts/MultiSigTokenWallet.sol | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index ebec928..f7817d8 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -11,6 +11,15 @@ contract MultiSigTokenWallet is MultiSigWallet { * Public functions * **/ + /** + * @dev only call parent constructor + */ + function MultiSigTokenWallet(address[] _owners, uint _required) + MultiSigWallet(_owners,_required) + public + { + //does nothing + } /** * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. * @param _token the token contract address @@ -105,7 +114,7 @@ contract MultiSigTokenWallet is MultiSigWallet { address _tokenAddr = _tokenList[i]; uint _amount = ERC20(_tokenAddr).balanceOf(address(this)); if (_amount > 0) { - ERC20(_tokenAddr).call(bytes4(keccak256("transfer(address,uint256)")), _dest, _amount); + ERC20(_tokenAddr).transfer(_dest, _amount); } } } From 72574d00dd6d86416dcdad39d2f3020e87db74df Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 13:07:07 -0200 Subject: [PATCH 11/34] simplified stub (only read ABI) --- contracts/MultiSigStub.sol | 62 +++++--------------------------------- 1 file changed, 7 insertions(+), 55 deletions(-) diff --git a/contracts/MultiSigStub.sol b/contracts/MultiSigStub.sol index 3bec475..9cf0d32 100644 --- a/contracts/MultiSigStub.sol +++ b/contracts/MultiSigStub.sol @@ -8,18 +8,6 @@ import "./DelegatedCall.sol"; */ contract MultiSigStub is DelegatedCall { - address[] public owners; - mapping (uint => Transaction) public transactions; - mapping (uint => mapping (address => bool)) public confirmations; - uint public transactionCount; - - struct Transaction { - address destination; - uint value; - bytes data; - bool executed; - } - function MultiSigStub(address[] _owners, uint256 _required) { //bytes4 sig = bytes4(sha3("constructor(address[],uint256)")); bytes4 sig = 0x36756a23; @@ -42,21 +30,6 @@ contract MultiSigStub is DelegatedCall { } - function submitTransaction(address destination, uint value, bytes data) - public - delegated - returns (uint) - { - - } - - function confirmTransaction(uint transactionId) - public - delegated - { - - } - /// @dev Returns the confirmation status of a transaction. /// @param transactionId Transaction ID. /// @return Confirmation status. @@ -103,9 +76,10 @@ contract MultiSigStub is DelegatedCall { function getOwners() public constant + delegated returns (address[]) { - return owners; + } /// @dev Returns array with owner addresses, which confirmed transaction. @@ -114,21 +88,10 @@ contract MultiSigStub is DelegatedCall { function getConfirmations(uint transactionId) public constant + delegated returns (address[] _confirmations) { - address[] memory confirmationsTemp = new address[](owners.length); - uint count = 0; - uint i; - for (i = 0; i < owners.length; i++) { - if (confirmations[transactionId][owners[i]]) { - confirmationsTemp[count] = owners[i]; - count += 1; - } - } - _confirmations = new address[](count); - for (i = 0; i < count; i++) { - _confirmations[i] = confirmationsTemp[i]; - } + } /// @dev Returns list of transaction IDs in defined range. @@ -137,24 +100,13 @@ contract MultiSigStub is DelegatedCall { /// @param pending Include pending transactions. /// @param executed Include executed transactions. /// @return Returns array of transaction IDs. - function getTransactionIds(uint from, uint to, bool pending, bool executed) + function getTransactionIds(uint from, uint to, bool pending, bool executed) public constant + delegated returns (uint[] _transactionIds) { - uint[] memory transactionIdsTemp = new uint[](transactionCount); - uint count = 0; - uint i; - for (i = 0; i < transactionCount; i++) { - if (pending && !transactions[i].executed || executed && transactions[i].executed) { - transactionIdsTemp[count] = i; - count += 1; - } - } - _transactionIds = new uint[](to - from); - for (i = from; i < to; i++) { - _transactionIds[i - from] = transactionIdsTemp[i]; - } + } function _getDelegatedContract() From 2442deada72763a382dea384b19926ebe9e21a49 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 13:12:13 -0200 Subject: [PATCH 12/34] add comment about hardcoded address --- contracts/MultiSigStub.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/MultiSigStub.sol b/contracts/MultiSigStub.sol index 9cf0d32..6c7451b 100644 --- a/contracts/MultiSigStub.sol +++ b/contracts/MultiSigStub.sol @@ -113,7 +113,7 @@ contract MultiSigStub is DelegatedCall { internal returns(address) { - return 0xCaFFE810d0dF52E27DC580AD4a3C6283B0094291; + return 0xCaFFE810d0dF52E27DC580AD4a3C6283B0094291; //hardcoded multinetwork address } } \ No newline at end of file From 1ac4789bf709c3c70144652bcd6e89237b35508d Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 13:50:54 -0200 Subject: [PATCH 13/34] fix solc version and comments --- contracts/MultiSigTokenWallet.sol | 7 ++++++- contracts/MultiSigWallet.sol | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index f7817d8..c988fca 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -1,8 +1,13 @@ -pragma solidity ^0.4.15; +pragma solidity ^0.4.18; import "./MultiSigWallet.sol"; import "./ERC20.sol"; +/** + * @title MultiSigTokenWallet + * @author Ricardo Guilherme Schmidt (Status Research & Development GmbH) + * MultiSigWallet that supports withdrawing all ERC20 tokens at once. + */ contract MultiSigTokenWallet is MultiSigWallet { event TokenDeposit(address indexed token, address indexed sender, uint value); diff --git a/contracts/MultiSigWallet.sol b/contracts/MultiSigWallet.sol index 642aef3..a6a70b4 100644 --- a/contracts/MultiSigWallet.sol +++ b/contracts/MultiSigWallet.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.17; +pragma solidity ^0.4.18; /// @title Multisignature wallet - Allows multiple parties to agree on transactions before execution. From 9aa000872d95c9c9b77ce1f888f13accdaef12f6 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 14:00:06 -0200 Subject: [PATCH 14/34] Add legacy support --- contracts/MultiSigTokenWalletV1.sol | 203 ++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 contracts/MultiSigTokenWalletV1.sol diff --git a/contracts/MultiSigTokenWalletV1.sol b/contracts/MultiSigTokenWalletV1.sol new file mode 100644 index 0000000..4375daf --- /dev/null +++ b/contracts/MultiSigTokenWalletV1.sol @@ -0,0 +1,203 @@ +pragma solidity ^0.4.15; + +import "./MultiSigWallet.sol"; +import "./ERC20.sol"; +// @dev This contract is deprecated, use the new version. +contract MultiSigTokenWalletV1 is MultiSigWallet { + + address[] public tokens; + mapping (address => uint) public tokenBalances; + mapping (address => address[]) public userList; + uint public nonce; + + event TokenDeposit(address _token, address indexed _sender, uint _value); + + /** + * Public functions + * + **/ + /** + * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. + * @param _token the token contract address + * @param _data might be used by child implementations + **/ + function depositToken(address _token, bytes _data) + public + { + address sender = msg.sender; + uint amount = ERC20(_token).allowance(sender, this); + deposit(sender, amount, _token, _data); + } + + /** + * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. + * @param _token the token contract address + * @param _data might be used by child implementations + **/ + function deposit(address _from, uint256 _amount, address _token, bytes _data) + public + { + if (_from == address(this)) + return; + uint _nonce = nonce; + bool result = ERC20(_token).transferFrom(_from, this, _amount); + assert(result); + //ERC23 not executed _deposited tokenFallback by + if (nonce == _nonce) { + _deposited(_from, _amount, _token, _data); + } + } + /** + * @notice watches for balance in a token contract + * @param _tokenAddr the token contract address + **/ + function watch(address _tokenAddr) + ownerExists(msg.sender) + { + uint oldBal = tokenBalances[_tokenAddr]; + uint newBal = ERC20(_tokenAddr).balanceOf(this); + if (newBal > oldBal) { + _deposited(0x0, newBal-oldBal, _tokenAddr, new bytes(0)); + } + } + + function setMyTokenList(address[] _tokenList) + public + { + userList[msg.sender] = _tokenList; + } + + function setTokenList(address[] _tokenList) + onlyWallet + { + tokens = _tokenList; + } + + /** + * @notice ERC23 Token fallback + * @param _from address incoming token + * @param _amount incoming amount + **/ + function tokenFallback(address _from, uint _amount, bytes _data) + public + { + _deposited(_from, _amount, msg.sender, _data); + } + + /** + * @notice Called MiniMeToken approvesAndCall to this contract, calls deposit. + * @param _from address incoming token + * @param _amount incoming amount + * @param _token the token contract address + * @param _data (might be used by child classes) + */ + function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) { + deposit(_from, _amount, _token, _data); + } + + /** + * @dev gives full ownership of this wallet to `_dest` removing older owners from wallet + * @param _dest the address of new controller + **/ + function releaseWallet(address _dest) + public + notNull(_dest) + ownerDoesNotExist(_dest) + onlyWallet + { + address[] memory _owners = owners; + uint numOwners = _owners.length; + addOwner(_dest); + for (uint i = 0; i < numOwners; i++) { + removeOwner(_owners[i]); + } + } + + /** + * @dev withdraw all recognized tokens balances and ether to `_dest` + * @param _dest the address of receiver + **/ + function withdrawEverything(address _dest) + public + notNull(_dest) + onlyWallet + { + withdrawAllTokens(_dest); + _dest.transfer(this.balance); + } + + /** + * @dev withdraw all recognized tokens balances to `_dest` + * @param _dest the address of receiver + **/ + function withdrawAllTokens(address _dest) + public + notNull(_dest) + onlyWallet + { + address[] memory _tokenList; + if (userList[_dest].length > 0) { + _tokenList = userList[_dest]; + } else { + _tokenList = tokens; + } + uint len = _tokenList.length; + for (uint i = 0;i < len; i++) { + address _tokenAddr = _tokenList[i]; + uint _amount = tokenBalances[_tokenAddr]; + if (_amount > 0) { + delete tokenBalances[_tokenAddr]; + ERC20(_tokenAddr).transfer(_dest, _amount); + } + } + } + + /** + * @dev withdraw `_tokenAddr` `_amount` to `_dest` + * @param _tokenAddr the address of the token + * @param _dest the address of receiver + * @param _amount the number of tokens to send + **/ + function withdrawToken(address _tokenAddr, address _dest, uint _amount) + public + notNull(_dest) + onlyWallet + { + require(_amount > 0); + uint _balance = tokenBalances[_tokenAddr]; + require(_amount <= _balance); + tokenBalances[_tokenAddr] = _balance - _amount; + bool result = ERC20(_tokenAddr).transfer(_dest, _amount); + assert(result); + } + + /** + * @dev register the deposit + **/ + function _deposited(address _from, uint _amount, address _tokenAddr, bytes) + internal + { + TokenDeposit(_tokenAddr,_from,_amount); + nonce++; + if (tokenBalances[_tokenAddr] == 0) { + tokens.push(_tokenAddr); + tokenBalances[_tokenAddr] = ERC20(_tokenAddr).balanceOf(this); + } else { + tokenBalances[_tokenAddr] += _amount; + } + } + + /* + * Web3 call functions + */ + /// @dev Returns list of tokens. + /// @return List of token addresses. + function getTokenList() + public + constant + returns (address[]) + { + return tokens; + } + +} \ No newline at end of file From fb2ed3d4007c7b8c18403e0f6bf38b0d5fe76771 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 14:00:45 -0200 Subject: [PATCH 15/34] lint --- contracts/MultiSigTokenWallet.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index c988fca..0a3a963 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -51,7 +51,6 @@ contract MultiSigTokenWallet is MultiSigWallet { bool result = ERC20(_token).transferFrom(_from, this, _amount); require(result); TokenDeposit(_token, _from, _amount); - } /** From 0678a18ec44ce902769f5c03dc23271fa7fbe4f8 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 14:33:01 -0200 Subject: [PATCH 16/34] lint --- contracts/DelegatedCall.sol | 2 +- contracts/ERC20.sol | 12 ++++++------ contracts/MultiSigFactory.sol | 2 +- contracts/MultiSigStub.sol | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol index 18b2851..6ae754c 100644 --- a/contracts/DelegatedCall.sol +++ b/contracts/DelegatedCall.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.12; +pragma solidity ^0.4.18; /** * @title DelegatedCall diff --git a/contracts/ERC20.sol b/contracts/ERC20.sol index e82eabb..fb47128 100644 --- a/contracts/ERC20.sol +++ b/contracts/ERC20.sol @@ -1,12 +1,12 @@ -pragma solidity ^0.4.15; +pragma solidity ^0.4.18; contract ERC20 { uint256 public totalSupply; - function balanceOf(address who) constant returns (uint256); - function allowance(address owner, address spender) constant returns (uint256); - function transfer(address to, uint256 value) returns (bool ok); - function transferFrom(address from, address to, uint256 value) returns (bool ok); - function approve(address spender, uint256 value) returns (bool ok); + function balanceOf(address who) public constant returns (uint256); + function allowance(address owner, address spender) public constant returns (uint256); + function transfer(address to, uint256 value) public returns (bool ok); + function transferFrom(address from, address to, uint256 value) public returns (bool ok); + function approve(address spender, uint256 value) public returns (bool ok); event Transfer(address indexed from, address indexed to, uint256 value); event Approval(address indexed owner, address indexed spender, uint256 value); } \ No newline at end of file diff --git a/contracts/MultiSigFactory.sol b/contracts/MultiSigFactory.sol index 99e80ae..30a5ac3 100644 --- a/contracts/MultiSigFactory.sol +++ b/contracts/MultiSigFactory.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.15; +pragma solidity ^0.4.18; import "./MultiSigStub.sol"; diff --git a/contracts/MultiSigStub.sol b/contracts/MultiSigStub.sol index 6c7451b..709bda0 100644 --- a/contracts/MultiSigStub.sol +++ b/contracts/MultiSigStub.sol @@ -1,9 +1,10 @@ -pragma solidity ^0.4.15; +pragma solidity ^0.4.18; import "./DelegatedCall.sol"; /** * @title MultiSigStub + * @author Ricardo Guilherme Schmidt (Status Research & Development GmbH) * Contract that delegates calls to a library to build a full MultiSigWallet that is cheap to create. */ contract MultiSigStub is DelegatedCall { From 622c3b3c5f89109902d75d14d57ad8cd5704e2da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Thu, 9 Nov 2017 16:44:06 +0000 Subject: [PATCH 17/34] Remove public_repo capability for admin-authorize-url --- src/clj/commiteth/github/core.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clj/commiteth/github/core.clj b/src/clj/commiteth/github/core.clj index c20d7ce..e4eadaf 100644 --- a/src/clj/commiteth/github/core.clj +++ b/src/clj/commiteth/github/core.clj @@ -44,7 +44,7 @@ (defn admin-authorize-url [] (if (github-app-enabled?) - (authorize-url "public_repo user:email") + (authorize-url "user:email") (authorize-url "admin:repo_hook repo user:email admin:org_hook"))) (defn access-settings-url [] From 106569c8d0e6cfed8df34f3d9b004ca0ddfb1d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Thu, 9 Nov 2017 17:10:01 +0000 Subject: [PATCH 18/34] Mock handle-installation and bump log level for installation event --- src/clj/commiteth/routes/services.clj | 1 + src/clj/commiteth/routes/webhooks.clj | 32 +++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/clj/commiteth/routes/services.clj b/src/clj/commiteth/routes/services.clj index 9b72051..47a9fec 100644 --- a/src/clj/commiteth/routes/services.clj +++ b/src/clj/commiteth/routes/services.clj @@ -68,6 +68,7 @@ :state 0 :hook_id nil})) +;; TODO(oskarth): We want to support these params via webhooks.clj/webhook-app route (defn handle-toggle-repo [user params can-create?] (log/debug "handle-toggle-repo" user params) (log/info "MANUAL ACTION REQUIRED: Possibly add repo:" (pr-str user) (pr-str params)) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 4f30e52..2d734c7 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -218,6 +218,31 @@ (handle-issue-reopened webhook-payload))) (ok)) +(defn handle-installation [{:keys [action installation sender]}] + ;; TODO(oskarth): Handle other installs? Like remove, say. + (when (= action "created") + (let [user-id (:id sender) + username (:login sender) + owner-avatar-url (:avatar_url sender) + ;; [{id,name,full_name},...] + repos (:repositories installation) + ;; TODO(oskarth): Handle install for each repo + first-repo (first repos) + ] + ;; NOTE(oskarth): All needed params to call equivalent + ;; handle-toggle-repo. + ;; DON'T need token cause we don't modify webhooks. + (log/info "handle-installation created" + (pr-str + {:user-id user-id + :name username + :owner-avatar-url owner-avatar-url + :all-selected-repos repos + :first-repo-id (:id first-repo) + :first-repo (:name first-repo) + :first-full-name (:full_name first-repo)})) + ;; TODO(oskarth): Actually install repo + ))) (defn handle-pull-request [pull-request] @@ -268,13 +293,16 @@ (log/debug "webhook-app POST, headers" headers) (let [raw-payload (slurp body) payload (json/parse-string raw-payload true)] - (log/debug "webhook-app POST, payload" payload) + (log/info "webhook-app POST, payload:" (pr-str payload)) (if (validate-secret-one-hook payload raw-payload (get headers "x-hub-signature")) (do (log/debug "Github secret validation OK app") - (log/debug "x-github-event app" (get headers "x-github-event")) + (log/info "x-github-event app" (get headers "x-github-event")) (case (get headers "x-github-event") "issues" (handle-issue payload) "pull_request" (handle-pull-request payload) + "installation" (handle-installation payload) (ok))) (forbidden))))) + + From a76a8ee402d2d55825ab86ce9adb26d4ec0eb2fa Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 16:46:46 -0200 Subject: [PATCH 19/34] fix warnings --- contracts/DelegatedCall.sol | 1 + contracts/MultiSigTokenWalletV1.sol | 6 +++++- contracts/TokenReg.sol | 29 ++++++++++++++--------------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol index 6ae754c..d132bfa 100644 --- a/contracts/DelegatedCall.sol +++ b/contracts/DelegatedCall.sol @@ -41,6 +41,7 @@ contract DelegatedCall { */ function _malloc(uint size) internal + pure returns(bytes32 ptr) { assembly { diff --git a/contracts/MultiSigTokenWalletV1.sol b/contracts/MultiSigTokenWalletV1.sol index 4375daf..4c19ba9 100644 --- a/contracts/MultiSigTokenWalletV1.sol +++ b/contracts/MultiSigTokenWalletV1.sol @@ -52,6 +52,7 @@ contract MultiSigTokenWalletV1 is MultiSigWallet { * @param _tokenAddr the token contract address **/ function watch(address _tokenAddr) + public ownerExists(msg.sender) { uint oldBal = tokenBalances[_tokenAddr]; @@ -68,6 +69,7 @@ contract MultiSigTokenWalletV1 is MultiSigWallet { } function setTokenList(address[] _tokenList) + public onlyWallet { tokens = _tokenList; @@ -91,7 +93,9 @@ contract MultiSigTokenWalletV1 is MultiSigWallet { * @param _token the token contract address * @param _data (might be used by child classes) */ - function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) { + function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) + public + { deposit(_from, _amount, _token, _data); } diff --git a/contracts/TokenReg.sol b/contracts/TokenReg.sol index 1c96736..778fb0a 100644 --- a/contracts/TokenReg.sol +++ b/contracts/TokenReg.sol @@ -2,7 +2,7 @@ //! By Gav Wood (Ethcore), 2016. //! Released under the Apache Licence 2. -pragma solidity ^0.4.0; +pragma solidity ^0.4.18; // From Owned.sol contract Owned { @@ -10,7 +10,7 @@ contract Owned { event NewOwner(address indexed old, address indexed current); - function setOwner(address _new) only_owner { NewOwner(owner, _new); owner = _new; } + function setOwner(address _new) public only_owner { NewOwner(owner, _new); owner = _new; } address public owner = msg.sender; } @@ -36,11 +36,11 @@ contract TokenReg is Owned { event Unregistered(string indexed tla, uint indexed id); event MetaChanged(uint indexed id, bytes32 indexed key, bytes32 value); - function register(address _addr, string _tla, uint _base, string _name) payable returns (bool) { + function register(address _addr, string _tla, uint _base, string _name) public payable returns (bool) { return registerAs(_addr, _tla, _base, _name, msg.sender); } - function registerAs(address _addr, string _tla, uint _base, string _name, address _owner) payable when_fee_paid when_address_free(_addr) when_is_tla(_tla) when_tla_free(_tla) returns (bool) { + function registerAs(address _addr, string _tla, uint _base, string _name, address _owner) public payable when_fee_paid when_address_free(_addr) when_is_tla(_tla) when_tla_free(_tla) returns (bool) { tokens.push(Token(_addr, _tla, _base, _name, _owner)); mapFromAddress[_addr] = tokens.length; mapFromTLA[_tla] = tokens.length; @@ -48,19 +48,19 @@ contract TokenReg is Owned { return true; } - function unregister(uint _id) only_owner { + function unregister(uint _id) public only_owner { Unregistered(tokens[_id].tla, _id); delete mapFromAddress[tokens[_id].addr]; delete mapFromTLA[tokens[_id].tla]; delete tokens[_id]; } - function setFee(uint _fee) only_owner { + function setFee(uint _fee) public only_owner { fee = _fee; } - function tokenCount() constant returns (uint) { return tokens.length; } - function token(uint _id) constant returns (address addr, string tla, uint base, string name, address owner) { + function tokenCount() public constant returns (uint) { return tokens.length; } + function token(uint _id) public constant returns (address addr, string tla, uint base, string name, address owner) { var t = tokens[_id]; addr = t.addr; tla = t.tla; @@ -69,7 +69,7 @@ contract TokenReg is Owned { owner = t.owner; } - function fromAddress(address _addr) constant returns (uint id, string tla, uint base, string name, address owner) { + function fromAddress(address _addr) public constant returns (uint id, string tla, uint base, string name, address owner) { id = mapFromAddress[_addr] - 1; var t = tokens[id]; tla = t.tla; @@ -78,7 +78,7 @@ contract TokenReg is Owned { owner = t.owner; } - function fromTLA(string _tla) constant returns (uint id, address addr, uint base, string name, address owner) { + function fromTLA(string _tla) public constant returns (uint id, address addr, uint base, string name, address owner) { id = mapFromTLA[_tla] - 1; var t = tokens[id]; addr = t.addr; @@ -87,18 +87,17 @@ contract TokenReg is Owned { owner = t.owner; } - function meta(uint _id, bytes32 _key) constant returns (bytes32) { + function meta(uint _id, bytes32 _key) public constant returns (bytes32) { return tokens[_id].meta[_key]; } - function setMeta(uint _id, bytes32 _key, bytes32 _value) only_token_owner(_id) { + function setMeta(uint _id, bytes32 _key, bytes32 _value) public only_token_owner(_id) { tokens[_id].meta[_key] = _value; MetaChanged(_id, _key, _value); } - function drain() only_owner { - if (!msg.sender.send(this.balance)) - throw; + function drain() public only_owner { + require (msg.sender.send(this.balance)); } mapping (address => uint) mapFromAddress; From a3931724123edb938d6fc7dd7fd42cace5f3ece8 Mon Sep 17 00:00:00 2001 From: Ricardo Guilherme Schmidt <3esmit@gmail.com> Date: Thu, 9 Nov 2017 17:30:13 -0200 Subject: [PATCH 20/34] fix rename to legacy --- src/clj/commiteth/eth/multisig_wallet.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/clj/commiteth/eth/multisig_wallet.clj b/src/clj/commiteth/eth/multisig_wallet.clj index 67bbb06..7e186bf 100644 --- a/src/clj/commiteth/eth/multisig_wallet.clj +++ b/src/clj/commiteth/eth/multisig_wallet.clj @@ -8,7 +8,7 @@ (:import [org.web3j abi.datatypes.Address abi.datatypes.generated.Uint256] - [commiteth.eth.contracts MultiSigTokenWallet])) + [commiteth.eth.contracts MultiSigTokenWalletV1])) (def method-ids (into {} @@ -126,7 +126,7 @@ (defn load-bounty-contract [addr] - (MultiSigTokenWallet/load addr + (MultiSigTokenWalletV1/load addr (create-web3j) (creds) (eth/gas-price) From edefb9883dcc195a24df65853c7dd19a1386c06e Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 10 Nov 2017 09:44:11 +0200 Subject: [PATCH 21/34] Revert "fix abi mismatch" --- src/clj/commiteth/eth/multisig_wallet.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/clj/commiteth/eth/multisig_wallet.clj b/src/clj/commiteth/eth/multisig_wallet.clj index 7e186bf..67bbb06 100644 --- a/src/clj/commiteth/eth/multisig_wallet.clj +++ b/src/clj/commiteth/eth/multisig_wallet.clj @@ -8,7 +8,7 @@ (:import [org.web3j abi.datatypes.Address abi.datatypes.generated.Uint256] - [commiteth.eth.contracts MultiSigTokenWalletV1])) + [commiteth.eth.contracts MultiSigTokenWallet])) (def method-ids (into {} @@ -126,7 +126,7 @@ (defn load-bounty-contract [addr] - (MultiSigTokenWalletV1/load addr + (MultiSigTokenWallet/load addr (create-web3j) (creds) (eth/gas-price) From 9bc86863129fb37df8d455f8e26fd1cd2163c208 Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 10 Nov 2017 09:45:18 +0200 Subject: [PATCH 22/34] Revert "fix warnings in contracts" --- contracts/DelegatedCall.sol | 1 - contracts/MultiSigTokenWalletV1.sol | 6 +----- contracts/TokenReg.sol | 29 +++++++++++++++-------------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol index d132bfa..6ae754c 100644 --- a/contracts/DelegatedCall.sol +++ b/contracts/DelegatedCall.sol @@ -41,7 +41,6 @@ contract DelegatedCall { */ function _malloc(uint size) internal - pure returns(bytes32 ptr) { assembly { diff --git a/contracts/MultiSigTokenWalletV1.sol b/contracts/MultiSigTokenWalletV1.sol index 4c19ba9..4375daf 100644 --- a/contracts/MultiSigTokenWalletV1.sol +++ b/contracts/MultiSigTokenWalletV1.sol @@ -52,7 +52,6 @@ contract MultiSigTokenWalletV1 is MultiSigWallet { * @param _tokenAddr the token contract address **/ function watch(address _tokenAddr) - public ownerExists(msg.sender) { uint oldBal = tokenBalances[_tokenAddr]; @@ -69,7 +68,6 @@ contract MultiSigTokenWalletV1 is MultiSigWallet { } function setTokenList(address[] _tokenList) - public onlyWallet { tokens = _tokenList; @@ -93,9 +91,7 @@ contract MultiSigTokenWalletV1 is MultiSigWallet { * @param _token the token contract address * @param _data (might be used by child classes) */ - function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) - public - { + function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) { deposit(_from, _amount, _token, _data); } diff --git a/contracts/TokenReg.sol b/contracts/TokenReg.sol index 778fb0a..1c96736 100644 --- a/contracts/TokenReg.sol +++ b/contracts/TokenReg.sol @@ -2,7 +2,7 @@ //! By Gav Wood (Ethcore), 2016. //! Released under the Apache Licence 2. -pragma solidity ^0.4.18; +pragma solidity ^0.4.0; // From Owned.sol contract Owned { @@ -10,7 +10,7 @@ contract Owned { event NewOwner(address indexed old, address indexed current); - function setOwner(address _new) public only_owner { NewOwner(owner, _new); owner = _new; } + function setOwner(address _new) only_owner { NewOwner(owner, _new); owner = _new; } address public owner = msg.sender; } @@ -36,11 +36,11 @@ contract TokenReg is Owned { event Unregistered(string indexed tla, uint indexed id); event MetaChanged(uint indexed id, bytes32 indexed key, bytes32 value); - function register(address _addr, string _tla, uint _base, string _name) public payable returns (bool) { + function register(address _addr, string _tla, uint _base, string _name) payable returns (bool) { return registerAs(_addr, _tla, _base, _name, msg.sender); } - function registerAs(address _addr, string _tla, uint _base, string _name, address _owner) public payable when_fee_paid when_address_free(_addr) when_is_tla(_tla) when_tla_free(_tla) returns (bool) { + function registerAs(address _addr, string _tla, uint _base, string _name, address _owner) payable when_fee_paid when_address_free(_addr) when_is_tla(_tla) when_tla_free(_tla) returns (bool) { tokens.push(Token(_addr, _tla, _base, _name, _owner)); mapFromAddress[_addr] = tokens.length; mapFromTLA[_tla] = tokens.length; @@ -48,19 +48,19 @@ contract TokenReg is Owned { return true; } - function unregister(uint _id) public only_owner { + function unregister(uint _id) only_owner { Unregistered(tokens[_id].tla, _id); delete mapFromAddress[tokens[_id].addr]; delete mapFromTLA[tokens[_id].tla]; delete tokens[_id]; } - function setFee(uint _fee) public only_owner { + function setFee(uint _fee) only_owner { fee = _fee; } - function tokenCount() public constant returns (uint) { return tokens.length; } - function token(uint _id) public constant returns (address addr, string tla, uint base, string name, address owner) { + function tokenCount() constant returns (uint) { return tokens.length; } + function token(uint _id) constant returns (address addr, string tla, uint base, string name, address owner) { var t = tokens[_id]; addr = t.addr; tla = t.tla; @@ -69,7 +69,7 @@ contract TokenReg is Owned { owner = t.owner; } - function fromAddress(address _addr) public constant returns (uint id, string tla, uint base, string name, address owner) { + function fromAddress(address _addr) constant returns (uint id, string tla, uint base, string name, address owner) { id = mapFromAddress[_addr] - 1; var t = tokens[id]; tla = t.tla; @@ -78,7 +78,7 @@ contract TokenReg is Owned { owner = t.owner; } - function fromTLA(string _tla) public constant returns (uint id, address addr, uint base, string name, address owner) { + function fromTLA(string _tla) constant returns (uint id, address addr, uint base, string name, address owner) { id = mapFromTLA[_tla] - 1; var t = tokens[id]; addr = t.addr; @@ -87,17 +87,18 @@ contract TokenReg is Owned { owner = t.owner; } - function meta(uint _id, bytes32 _key) public constant returns (bytes32) { + function meta(uint _id, bytes32 _key) constant returns (bytes32) { return tokens[_id].meta[_key]; } - function setMeta(uint _id, bytes32 _key, bytes32 _value) public only_token_owner(_id) { + function setMeta(uint _id, bytes32 _key, bytes32 _value) only_token_owner(_id) { tokens[_id].meta[_key] = _value; MetaChanged(_id, _key, _value); } - function drain() public only_owner { - require (msg.sender.send(this.balance)); + function drain() only_owner { + if (!msg.sender.send(this.balance)) + throw; } mapping (address => uint) mapFromAddress; From 2de8d77404c73501be838cfd643e04a8835a4b14 Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 10 Nov 2017 09:46:04 +0200 Subject: [PATCH 23/34] Revert "Improved Multisig" --- contracts/DelegatedCall.sol | 72 ---- contracts/ERC20.sol | 12 +- contracts/MultiSigFactory.sol | 7 +- contracts/MultiSigStub.sol | 142 +++++++- contracts/MultiSigTokenWallet.sol | 511 +++++++++++++++++++++++++--- contracts/MultiSigTokenWalletV1.sol | 203 ----------- contracts/MultiSigWallet.sol | 378 -------------------- 7 files changed, 602 insertions(+), 723 deletions(-) delete mode 100644 contracts/DelegatedCall.sol delete mode 100644 contracts/MultiSigTokenWalletV1.sol delete mode 100644 contracts/MultiSigWallet.sol diff --git a/contracts/DelegatedCall.sol b/contracts/DelegatedCall.sol deleted file mode 100644 index 6ae754c..0000000 --- a/contracts/DelegatedCall.sol +++ /dev/null @@ -1,72 +0,0 @@ -pragma solidity ^0.4.18; - -/** - * @title DelegatedCall - * @author Ricardo Guilherme Schmidt (Status Research & Development GmbH) - * Abstract contract that delegates all calls to contract returned by abstract function `_getDelegatedContract` - */ -contract DelegatedCall { - - /** - * @dev delegates the call of this function - */ - modifier delegated - { - uint inSize = msg.data.length; - bytes32 inDataPtr = _malloc(inSize); - - assembly { - calldatacopy(inDataPtr, 0x0, inSize) - } - - bytes32 outDataPtr; - uint256 outSize; - - (outDataPtr, outSize) = _delegatecall(inDataPtr, inSize); - _; - assembly { - return(outDataPtr, outSize) - } - } - - /** - * @dev defines the address for delegation of calls - */ - function _getDelegatedContract() - internal - returns(address); - - /** - * @dev allocates memory to a pointer - */ - function _malloc(uint size) - internal - returns(bytes32 ptr) - { - assembly { - ptr := mload(0x40) - mstore(0x40, add(ptr, size)) - } - } - - /** - * @dev delegates the data in pointer - */ - function _delegatecall(bytes32 inDataPtr, uint inSize) - internal - returns(bytes32 outDataPtr, uint256 outSize) - { - address target = _getDelegatedContract(); - bool failed; - assembly { - failed := iszero(delegatecall(sub(gas, 10000), target, inDataPtr, inSize, 0, 0)) - outSize := returndatasize - } - require(!failed); - outDataPtr = _malloc(outSize); - assembly { - returndatacopy(outDataPtr, 0, outSize) - } - } - -} diff --git a/contracts/ERC20.sol b/contracts/ERC20.sol index fb47128..e82eabb 100644 --- a/contracts/ERC20.sol +++ b/contracts/ERC20.sol @@ -1,12 +1,12 @@ -pragma solidity ^0.4.18; +pragma solidity ^0.4.15; contract ERC20 { uint256 public totalSupply; - function balanceOf(address who) public constant returns (uint256); - function allowance(address owner, address spender) public constant returns (uint256); - function transfer(address to, uint256 value) public returns (bool ok); - function transferFrom(address from, address to, uint256 value) public returns (bool ok); - function approve(address spender, uint256 value) public returns (bool ok); + function balanceOf(address who) constant returns (uint256); + function allowance(address owner, address spender) constant returns (uint256); + function transfer(address to, uint256 value) returns (bool ok); + function transferFrom(address from, address to, uint256 value) returns (bool ok); + function approve(address spender, uint256 value) returns (bool ok); event Transfer(address indexed from, address indexed to, uint256 value); event Approval(address indexed owner, address indexed spender, uint256 value); } \ No newline at end of file diff --git a/contracts/MultiSigFactory.sol b/contracts/MultiSigFactory.sol index 30a5ac3..62bc00a 100644 --- a/contracts/MultiSigFactory.sol +++ b/contracts/MultiSigFactory.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.18; +pragma solidity ^0.4.15; import "./MultiSigStub.sol"; @@ -6,10 +6,7 @@ contract MultiSigFactory { event Create(address indexed caller, address createdContract); - function create(address[] owners, uint256 required) - public - returns (address wallet) - { + function create(address[] owners, uint256 required) returns (address wallet) { wallet = new MultiSigStub(owners, required); Create(msg.sender, wallet); } diff --git a/contracts/MultiSigStub.sol b/contracts/MultiSigStub.sol index 709bda0..4857129 100644 --- a/contracts/MultiSigStub.sol +++ b/contracts/MultiSigStub.sol @@ -1,14 +1,24 @@ -pragma solidity ^0.4.18; - -import "./DelegatedCall.sol"; +pragma solidity ^0.4.15; /** * @title MultiSigStub - * @author Ricardo Guilherme Schmidt (Status Research & Development GmbH) * Contract that delegates calls to a library to build a full MultiSigWallet that is cheap to create. */ -contract MultiSigStub is DelegatedCall { +contract MultiSigStub { + address[] public owners; + address[] public tokens; + mapping (uint => Transaction) public transactions; + mapping (uint => mapping (address => bool)) public confirmations; + uint public transactionCount; + + struct Transaction { + address destination; + uint value; + bytes data; + bool executed; + } + function MultiSigStub(address[] _owners, uint256 _required) { //bytes4 sig = bytes4(sha3("constructor(address[],uint256)")); bytes4 sig = 0x36756a23; @@ -24,6 +34,21 @@ contract MultiSigStub is DelegatedCall { _delegatecall(mData, size); } + modifier delegated { + uint size = msg.data.length; + bytes32 mData = _malloc(size); + + assembly { + calldatacopy(mData, 0x0, size) + } + + bytes32 mResult = _delegatecall(mData, size); + _; + assembly { + return(mResult, 0x20) + } + } + function() payable delegated @@ -31,6 +56,34 @@ contract MultiSigStub is DelegatedCall { } + function submitTransaction(address destination, uint value, bytes data) + public + delegated + returns (uint) + { + + } + + function confirmTransaction(uint transactionId) + public + delegated + { + + } + + function watch(address _tokenAddr) + public + delegated + { + + } + + function setMyTokenList(address[] _tokenList) + public + delegated + { + + } /// @dev Returns the confirmation status of a transaction. /// @param transactionId Transaction ID. /// @return Confirmation status. @@ -46,6 +99,15 @@ contract MultiSigStub is DelegatedCall { /* * Web3 call functions */ + function tokenBalances(address tokenAddress) + public + constant + delegated + returns (uint) + { + + } + /// @dev Returns number of confirmations of a transaction. /// @param transactionId Transaction ID. @@ -77,10 +139,19 @@ contract MultiSigStub is DelegatedCall { function getOwners() public constant - delegated returns (address[]) { + return owners; + } + /// @dev Returns list of tokens. + /// @return List of token addresses. + function getTokenList() + public + constant + returns (address[]) + { + return tokens; } /// @dev Returns array with owner addresses, which confirmed transaction. @@ -89,10 +160,21 @@ contract MultiSigStub is DelegatedCall { function getConfirmations(uint transactionId) public constant - delegated returns (address[] _confirmations) { - + address[] memory confirmationsTemp = new address[](owners.length); + uint count = 0; + uint i; + for (i = 0; i < owners.length; i++) { + if (confirmations[transactionId][owners[i]]) { + confirmationsTemp[count] = owners[i]; + count += 1; + } + } + _confirmations = new address[](count); + for (i = 0; i < count; i++) { + _confirmations[i] = confirmationsTemp[i]; + } } /// @dev Returns list of transaction IDs in defined range. @@ -101,20 +183,50 @@ contract MultiSigStub is DelegatedCall { /// @param pending Include pending transactions. /// @param executed Include executed transactions. /// @return Returns array of transaction IDs. - function getTransactionIds(uint from, uint to, bool pending, bool executed) + function getTransactionIds(uint from, uint to, bool pending, bool executed) public constant - delegated returns (uint[] _transactionIds) { - + uint[] memory transactionIdsTemp = new uint[](transactionCount); + uint count = 0; + uint i; + for (i = 0; i < transactionCount; i++) { + if (pending && !transactions[i].executed || executed && transactions[i].executed) { + transactionIdsTemp[count] = i; + count += 1; + } + } + _transactionIds = new uint[](to - from); + for (i = from; i < to; i++) { + _transactionIds[i - from] = transactionIdsTemp[i]; + } } - function _getDelegatedContract() - internal - returns(address) + + function _malloc(uint size) + private + returns(bytes32 mData) { - return 0xCaFFE810d0dF52E27DC580AD4a3C6283B0094291; //hardcoded multinetwork address + assembly { + mData := mload(0x40) + mstore(0x40, add(mData, size)) + } + } + + function _delegatecall(bytes32 mData, uint size) + private + returns(bytes32 mResult) + { + address target = 0xc0FFeEE61948d8993864a73a099c0E38D887d3F4; //Multinetwork + mResult = _malloc(32); + bool failed; + + assembly { + failed := iszero(delegatecall(sub(gas, 10000), target, mData, size, mResult, 0x20)) + } + + assert(!failed); } } \ No newline at end of file diff --git a/contracts/MultiSigTokenWallet.sol b/contracts/MultiSigTokenWallet.sol index 0a3a963..d1f12f9 100644 --- a/contracts/MultiSigTokenWallet.sol +++ b/contracts/MultiSigTokenWallet.sol @@ -1,30 +1,114 @@ -pragma solidity ^0.4.18; +pragma solidity ^0.4.15; -import "./MultiSigWallet.sol"; import "./ERC20.sol"; -/** - * @title MultiSigTokenWallet - * @author Ricardo Guilherme Schmidt (Status Research & Development GmbH) - * MultiSigWallet that supports withdrawing all ERC20 tokens at once. - */ -contract MultiSigTokenWallet is MultiSigWallet { +contract MultiSigTokenWallet { - event TokenDeposit(address indexed token, address indexed sender, uint value); + address[] public owners; + address[] public tokens; + mapping (uint => Transaction) public transactions; + mapping (uint => mapping (address => bool)) public confirmations; + uint public transactionCount; + + mapping (address => uint) public tokenBalances; + mapping (address => bool) public isOwner; + mapping (address => address[]) public userList; + uint public required; + uint public nonce; + + struct Transaction { + address destination; + uint value; + bytes data; + bool executed; + } + + uint constant public MAX_OWNER_COUNT = 50; + + event Confirmation(address indexed _sender, uint indexed _transactionId); + event Revocation(address indexed _sender, uint indexed _transactionId); + event Submission(uint indexed _transactionId); + event Execution(uint indexed _transactionId); + event ExecutionFailure(uint indexed _transactionId); + event Deposit(address indexed _sender, uint _value); + event TokenDeposit(address _token, address indexed _sender, uint _value); + event OwnerAddition(address indexed _owner); + event OwnerRemoval(address indexed _owner); + event RequirementChange(uint _required); + + modifier onlyWallet() { + require (msg.sender == address(this)); + _; + } + + modifier ownerDoesNotExist(address owner) { + require (!isOwner[owner]); + _; + } + + modifier ownerExists(address owner) { + require (isOwner[owner]); + _; + } + + modifier transactionExists(uint transactionId) { + require (transactions[transactionId].destination != 0); + _; + } + + modifier confirmed(uint transactionId, address owner) { + require (confirmations[transactionId][owner]); + _; + } + + modifier notConfirmed(uint transactionId, address owner) { + require(!confirmations[transactionId][owner]); + _; + } + + modifier notExecuted(uint transactionId) { + require (!transactions[transactionId].executed); + _; + } + + modifier notNull(address _address) { + require (_address != 0); + _; + } + + modifier validRequirement(uint ownerCount, uint _required) { + require (ownerCount <= MAX_OWNER_COUNT && _required <= ownerCount && _required != 0 && ownerCount != 0); + _; + } + + /// @dev Fallback function allows to deposit ether. + function() + payable + { + if (msg.value > 0) + Deposit(msg.sender, msg.value); + } /** * Public functions * **/ - /** - * @dev only call parent constructor - */ - function MultiSigTokenWallet(address[] _owners, uint _required) - MultiSigWallet(_owners,_required) - public + /// @dev Contract constructor sets initial owners and required number of confirmations. + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + function constructor(address[] _owners, uint _required) + public + validRequirement(_owners.length, _required) { - //does nothing + require(owners.length == 0 && required == 0); + for (uint i = 0; i < _owners.length; i++) { + require(!isOwner[_owners[i]] && _owners[i] != 0); + isOwner[_owners[i]] = true; + } + owners = _owners; + required = _required; } + /** * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. * @param _token the token contract address @@ -48,26 +132,49 @@ contract MultiSigTokenWallet is MultiSigWallet { { if (_from == address(this)) return; + uint _nonce = nonce; bool result = ERC20(_token).transferFrom(_from, this, _amount); - require(result); - TokenDeposit(_token, _from, _amount); + assert(result); + //ERC23 not executed _deposited tokenFallback by + if (nonce == _nonce) { + _deposited(_from, _amount, _token, _data); + } + } + /** + * @notice watches for balance in a token contract + * @param _tokenAddr the token contract address + **/ + function watch(address _tokenAddr) + ownerExists(msg.sender) + { + uint oldBal = tokenBalances[_tokenAddr]; + uint newBal = ERC20(_tokenAddr).balanceOf(this); + if (newBal > oldBal) { + _deposited(0x0, newBal-oldBal, _tokenAddr, new bytes(0)); + } } + function setMyTokenList(address[] _tokenList) + public + { + userList[msg.sender] = _tokenList; + } + + function setTokenList(address[] _tokenList) + onlyWallet + { + tokens = _tokenList; + } + /** * @notice ERC23 Token fallback * @param _from address incoming token * @param _amount incoming amount **/ - function tokenFallback( - address _from, - uint _amount, - bytes _data - ) + function tokenFallback(address _from, uint _amount, bytes _data) public - returns (bool) { - TokenDeposit(msg.sender, _from, _amount); - return true; + _deposited(_from, _amount, msg.sender, _data); } /** @@ -77,50 +184,366 @@ contract MultiSigTokenWallet is MultiSigWallet { * @param _token the token contract address * @param _data (might be used by child classes) */ - function receiveApproval( - address _from, - uint256 _amount, - address _token, - bytes _data - ) - public - { + function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) { deposit(_from, _amount, _token, _data); } + /// @dev Allows to add a new owner. Transaction has to be sent by wallet. + /// @param owner Address of new owner. + function addOwner(address owner) + public + onlyWallet + ownerDoesNotExist(owner) + notNull(owner) + validRequirement(owners.length + 1, required) + { + isOwner[owner] = true; + owners.push(owner); + OwnerAddition(owner); + } + + /// @dev Allows to remove an owner. Transaction has to be sent by wallet. + /// @param owner Address of owner. + function removeOwner(address owner) + public + onlyWallet + ownerExists(owner) + { + isOwner[owner] = false; + uint _len = owners.length - 1; + for (uint i = 0; i < _len; i++) { + if (owners[i] == owner) { + owners[i] = owners[owners.length - 1]; + break; + } + } + owners.length -= 1; + if (required > owners.length) + changeRequirement(owners.length); + OwnerRemoval(owner); + } + + /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet. + /// @param owner Address of owner to be replaced. + /// @param owner Address of new owner. + function replaceOwner(address owner, address newOwner) + public + onlyWallet + ownerExists(owner) + ownerDoesNotExist(newOwner) + { + for (uint i = 0; i < owners.length; i++) { + if (owners[i] == owner) { + owners[i] = newOwner; + break; + } + } + isOwner[owner] = false; + isOwner[newOwner] = true; + OwnerRemoval(owner); + OwnerAddition(newOwner); + } + /** - * @dev withdraw all tokens in list and ether to `_dest` - * @param _dest the address of receiver - * @param _tokenList the list of tokens to withdraw all balance + * @dev gives full ownership of this wallet to `_dest` removing older owners from wallet + * @param _dest the address of new controller **/ - function withdrawEverything(address _dest, address[] _tokenList) + function releaseWallet(address _dest) + public + notNull(_dest) + ownerDoesNotExist(_dest) + onlyWallet + { + address[] memory _owners = owners; + uint numOwners = _owners.length; + addOwner(_dest); + for (uint i = 0; i < numOwners; i++) { + removeOwner(_owners[i]); + } + } + + /// @dev Allows to change the number of required confirmations. Transaction has to be sent by wallet. + /// @param _required Number of required confirmations. + function changeRequirement(uint _required) + public + onlyWallet + validRequirement(owners.length, _required) + { + required = _required; + RequirementChange(_required); + } + + /// @dev Allows an owner to submit and confirm a transaction. + /// @param destination Transaction target address. + /// @param value Transaction ether value. + /// @param data Transaction data payload. + /// @return Returns transaction ID. + function submitTransaction(address destination, uint value, bytes data) + public + returns (uint transactionId) + { + transactionId = addTransaction(destination, value, data); + confirmTransaction(transactionId); + } + + /// @dev Allows an owner to confirm a transaction. + /// @param transactionId Transaction ID. + function confirmTransaction(uint transactionId) + public + ownerExists(msg.sender) + transactionExists(transactionId) + notConfirmed(transactionId, msg.sender) + { + confirmations[transactionId][msg.sender] = true; + Confirmation(msg.sender, transactionId); + executeTransaction(transactionId); + } + + /// @dev Allows an owner to revoke a confirmation for a transaction. + /// @param transactionId Transaction ID. + function revokeConfirmation(uint transactionId) + public + ownerExists(msg.sender) + confirmed(transactionId, msg.sender) + notExecuted(transactionId) + { + confirmations[transactionId][msg.sender] = false; + Revocation(msg.sender, transactionId); + } + + /// @dev Allows anyone to execute a confirmed transaction. + /// @param transactionId Transaction ID. + function executeTransaction(uint transactionId) + public + notExecuted(transactionId) + { + if (isConfirmed(transactionId)) { + Transaction storage txx = transactions[transactionId]; + txx.executed = true; + if (txx.destination.call.value(txx.value)(txx.data)) { + Execution(transactionId); + } else { + ExecutionFailure(transactionId); + txx.executed = false; + } + } + } + + /** + * @dev withdraw all recognized tokens balances and ether to `_dest` + * @param _dest the address of receiver + **/ + function withdrawEverything(address _dest) public notNull(_dest) onlyWallet { - withdrawAllTokens(_dest, _tokenList); + withdrawAllTokens(_dest); _dest.transfer(this.balance); } /** - * @dev withdraw all listed tokens balances to `_dest` + * @dev withdraw all recognized tokens balances to `_dest` * @param _dest the address of receiver - * @param _tokenList the list of tokens to withdraw all balance **/ - function withdrawAllTokens(address _dest, address[] _tokenList) + function withdrawAllTokens(address _dest) public notNull(_dest) onlyWallet { + address[] memory _tokenList; + if (userList[_dest].length > 0) { + _tokenList = userList[_dest]; + } else { + _tokenList = tokens; + } uint len = _tokenList.length; for (uint i = 0;i < len; i++) { address _tokenAddr = _tokenList[i]; - uint _amount = ERC20(_tokenAddr).balanceOf(address(this)); + uint _amount = tokenBalances[_tokenAddr]; if (_amount > 0) { + delete tokenBalances[_tokenAddr]; ERC20(_tokenAddr).transfer(_dest, _amount); } } } -} \ No newline at end of file + /** + * @dev withdraw `_tokenAddr` `_amount` to `_dest` + * @param _tokenAddr the address of the token + * @param _dest the address of receiver + * @param _amount the number of tokens to send + **/ + function withdrawToken(address _tokenAddr, address _dest, uint _amount) + public + notNull(_dest) + onlyWallet + { + require(_amount > 0); + uint _balance = tokenBalances[_tokenAddr]; + require(_amount <= _balance); + tokenBalances[_tokenAddr] = _balance - _amount; + bool result = ERC20(_tokenAddr).transfer(_dest, _amount); + assert(result); + } + + /// @dev Returns the confirmation status of a transaction. + /// @param transactionId Transaction ID. + /// @return Confirmation status. + function isConfirmed(uint transactionId) + public + constant + returns (bool) + { + uint count = 0; + for (uint i = 0; i < owners.length; i++) { + if (confirmations[transactionId][owners[i]]) + count += 1; + if (count == required) + return true; + } + } + + /* + * Internal functions + */ + /// @dev Adds a new transaction to the transaction mapping, if transaction does not exist yet. + /// @param destination Transaction target address. + /// @param value Transaction ether value. + /// @param data Transaction data payload. + /// @return Returns transaction ID. + function addTransaction(address destination, uint value, bytes data) + internal + notNull(destination) + returns (uint transactionId) + { + transactionId = transactionCount; + transactions[transactionId] = Transaction({ + destination: destination, + value: value, + data: data, + executed: false + }); + transactionCount += 1; + Submission(transactionId); + } + + /** + * @dev register the deposit + **/ + function _deposited(address _from, uint _amount, address _tokenAddr, bytes) + internal + { + TokenDeposit(_tokenAddr,_from,_amount); + nonce++; + if (tokenBalances[_tokenAddr] == 0) { + tokens.push(_tokenAddr); + tokenBalances[_tokenAddr] = ERC20(_tokenAddr).balanceOf(this); + } else { + tokenBalances[_tokenAddr] += _amount; + } + } + + /* + * Web3 call functions + */ + /// @dev Returns number of confirmations of a transaction. + /// @param transactionId Transaction ID. + /// @return Number of confirmations. + function getConfirmationCount(uint transactionId) + public + constant + returns (uint count) + { + for (uint i = 0; i < owners.length; i++) { + if (confirmations[transactionId][owners[i]]) + count += 1; + } + } + + /// @dev Returns total number of transactions after filters are applied. + /// @param pending Include pending transactions. + /// @param executed Include executed transactions. + /// @return Total number of transactions after filters are applied. + function getTransactionCount(bool pending, bool executed) + public + constant + returns (uint count) + { + for (uint i = 0; i < transactionCount; i++) { + if (pending && !transactions[i].executed || executed && transactions[i].executed) + count += 1; + } + } + + /// @dev Returns list of owners. + /// @return List of owner addresses. + function getOwners() + public + constant + returns (address[]) + { + return owners; + } + + /// @dev Returns list of tokens. + /// @return List of token addresses. + function getTokenList() + public + constant + returns (address[]) + { + return tokens; + } + + /// @dev Returns array with owner addresses, which confirmed transaction. + /// @param transactionId Transaction ID. + /// @return Returns array of owner addresses. + function getConfirmations(uint transactionId) + public + constant + returns (address[] _confirmations) + { + address[] memory confirmationsTemp = new address[](owners.length); + uint count = 0; + uint i; + for (i = 0; i < owners.length; i++) { + if (confirmations[transactionId][owners[i]]) { + confirmationsTemp[count] = owners[i]; + count += 1; + } + } + _confirmations = new address[](count); + for (i = 0; i < count; i++) { + _confirmations[i] = confirmationsTemp[i]; + } + } + + /// @dev Returns list of transaction IDs in defined range. + /// @param from Index start position of transaction array. + /// @param to Index end position of transaction array. + /// @param pending Include pending transactions. + /// @param executed Include executed transactions. + /// @return Returns array of transaction IDs. + function getTransactionIds(uint from, uint to, bool pending, bool executed) + public + constant + returns (uint[] _transactionIds) + { + uint[] memory transactionIdsTemp = new uint[](transactionCount); + uint count = 0; + uint i; + for (i = 0; i < transactionCount; i++) { + if (pending && !transactions[i].executed || executed && transactions[i].executed) { + transactionIdsTemp[count] = i; + count += 1; + } + } + _transactionIds = new uint[](to - from); + for (i = from; i < to; i++) { + _transactionIds[i - from] = transactionIdsTemp[i]; + } + } + +} diff --git a/contracts/MultiSigTokenWalletV1.sol b/contracts/MultiSigTokenWalletV1.sol deleted file mode 100644 index 4375daf..0000000 --- a/contracts/MultiSigTokenWalletV1.sol +++ /dev/null @@ -1,203 +0,0 @@ -pragma solidity ^0.4.15; - -import "./MultiSigWallet.sol"; -import "./ERC20.sol"; -// @dev This contract is deprecated, use the new version. -contract MultiSigTokenWalletV1 is MultiSigWallet { - - address[] public tokens; - mapping (address => uint) public tokenBalances; - mapping (address => address[]) public userList; - uint public nonce; - - event TokenDeposit(address _token, address indexed _sender, uint _value); - - /** - * Public functions - * - **/ - /** - * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. - * @param _token the token contract address - * @param _data might be used by child implementations - **/ - function depositToken(address _token, bytes _data) - public - { - address sender = msg.sender; - uint amount = ERC20(_token).allowance(sender, this); - deposit(sender, amount, _token, _data); - } - - /** - * @notice deposit a ERC20 token. The amount of deposit is the allowance set to this contract. - * @param _token the token contract address - * @param _data might be used by child implementations - **/ - function deposit(address _from, uint256 _amount, address _token, bytes _data) - public - { - if (_from == address(this)) - return; - uint _nonce = nonce; - bool result = ERC20(_token).transferFrom(_from, this, _amount); - assert(result); - //ERC23 not executed _deposited tokenFallback by - if (nonce == _nonce) { - _deposited(_from, _amount, _token, _data); - } - } - /** - * @notice watches for balance in a token contract - * @param _tokenAddr the token contract address - **/ - function watch(address _tokenAddr) - ownerExists(msg.sender) - { - uint oldBal = tokenBalances[_tokenAddr]; - uint newBal = ERC20(_tokenAddr).balanceOf(this); - if (newBal > oldBal) { - _deposited(0x0, newBal-oldBal, _tokenAddr, new bytes(0)); - } - } - - function setMyTokenList(address[] _tokenList) - public - { - userList[msg.sender] = _tokenList; - } - - function setTokenList(address[] _tokenList) - onlyWallet - { - tokens = _tokenList; - } - - /** - * @notice ERC23 Token fallback - * @param _from address incoming token - * @param _amount incoming amount - **/ - function tokenFallback(address _from, uint _amount, bytes _data) - public - { - _deposited(_from, _amount, msg.sender, _data); - } - - /** - * @notice Called MiniMeToken approvesAndCall to this contract, calls deposit. - * @param _from address incoming token - * @param _amount incoming amount - * @param _token the token contract address - * @param _data (might be used by child classes) - */ - function receiveApproval(address _from, uint256 _amount, address _token, bytes _data) { - deposit(_from, _amount, _token, _data); - } - - /** - * @dev gives full ownership of this wallet to `_dest` removing older owners from wallet - * @param _dest the address of new controller - **/ - function releaseWallet(address _dest) - public - notNull(_dest) - ownerDoesNotExist(_dest) - onlyWallet - { - address[] memory _owners = owners; - uint numOwners = _owners.length; - addOwner(_dest); - for (uint i = 0; i < numOwners; i++) { - removeOwner(_owners[i]); - } - } - - /** - * @dev withdraw all recognized tokens balances and ether to `_dest` - * @param _dest the address of receiver - **/ - function withdrawEverything(address _dest) - public - notNull(_dest) - onlyWallet - { - withdrawAllTokens(_dest); - _dest.transfer(this.balance); - } - - /** - * @dev withdraw all recognized tokens balances to `_dest` - * @param _dest the address of receiver - **/ - function withdrawAllTokens(address _dest) - public - notNull(_dest) - onlyWallet - { - address[] memory _tokenList; - if (userList[_dest].length > 0) { - _tokenList = userList[_dest]; - } else { - _tokenList = tokens; - } - uint len = _tokenList.length; - for (uint i = 0;i < len; i++) { - address _tokenAddr = _tokenList[i]; - uint _amount = tokenBalances[_tokenAddr]; - if (_amount > 0) { - delete tokenBalances[_tokenAddr]; - ERC20(_tokenAddr).transfer(_dest, _amount); - } - } - } - - /** - * @dev withdraw `_tokenAddr` `_amount` to `_dest` - * @param _tokenAddr the address of the token - * @param _dest the address of receiver - * @param _amount the number of tokens to send - **/ - function withdrawToken(address _tokenAddr, address _dest, uint _amount) - public - notNull(_dest) - onlyWallet - { - require(_amount > 0); - uint _balance = tokenBalances[_tokenAddr]; - require(_amount <= _balance); - tokenBalances[_tokenAddr] = _balance - _amount; - bool result = ERC20(_tokenAddr).transfer(_dest, _amount); - assert(result); - } - - /** - * @dev register the deposit - **/ - function _deposited(address _from, uint _amount, address _tokenAddr, bytes) - internal - { - TokenDeposit(_tokenAddr,_from,_amount); - nonce++; - if (tokenBalances[_tokenAddr] == 0) { - tokens.push(_tokenAddr); - tokenBalances[_tokenAddr] = ERC20(_tokenAddr).balanceOf(this); - } else { - tokenBalances[_tokenAddr] += _amount; - } - } - - /* - * Web3 call functions - */ - /// @dev Returns list of tokens. - /// @return List of token addresses. - function getTokenList() - public - constant - returns (address[]) - { - return tokens; - } - -} \ No newline at end of file diff --git a/contracts/MultiSigWallet.sol b/contracts/MultiSigWallet.sol deleted file mode 100644 index a6a70b4..0000000 --- a/contracts/MultiSigWallet.sol +++ /dev/null @@ -1,378 +0,0 @@ -pragma solidity ^0.4.18; - - -/// @title Multisignature wallet - Allows multiple parties to agree on transactions before execution. -/// @author Stefan George - -contract MultiSigWallet { - - /* - * Events - */ - event Confirmation(address indexed sender, uint indexed transactionId); - event Revocation(address indexed sender, uint indexed transactionId); - event Submission(uint indexed transactionId); - event Execution(uint indexed transactionId); - event ExecutionFailure(uint indexed transactionId); - event Deposit(address indexed sender, uint value); - event OwnerAddition(address indexed owner); - event OwnerRemoval(address indexed owner); - event RequirementChange(uint required); - - /* - * Constants - */ - uint constant public MAX_OWNER_COUNT = 50; - - /* - * Storage - */ - mapping (uint => Transaction) public transactions; - mapping (uint => mapping (address => bool)) public confirmations; - mapping (address => bool) public isOwner; - address[] public owners; - uint public required; - uint public transactionCount; - - struct Transaction { - address destination; - uint value; - bytes data; - bool executed; - } - - /* - * Modifiers - */ - modifier onlyWallet() { - require (msg.sender == address(this)); - _; - } - - modifier ownerDoesNotExist(address owner) { - require (!isOwner[owner]); - _; - } - - modifier ownerExists(address owner) { - require (isOwner[owner]); - _; - } - - modifier transactionExists(uint transactionId) { - require (transactions[transactionId].destination != 0); - _; - } - - modifier confirmed(uint transactionId, address owner) { - require (confirmations[transactionId][owner]); - _; - } - - modifier notConfirmed(uint transactionId, address owner) { - require(!confirmations[transactionId][owner]); - _; - } - - modifier notExecuted(uint transactionId) { - require (!transactions[transactionId].executed); - _; - } - - modifier notNull(address _address) { - require (_address != 0); - _; - } - - modifier validRequirement(uint ownerCount, uint _required) { - require (ownerCount <= MAX_OWNER_COUNT && _required <= ownerCount && _required != 0 && ownerCount != 0); - _; - } - - /// @dev Fallback function allows to deposit ether. - function () - public - payable - { - if (msg.value > 0) - Deposit(msg.sender, msg.value); - } - - /* - * Public functions - */ - /// @dev Constructor calls function `constructor(address[],uint256)` - /// @param _owners List of initial owners. - /// @param _required Number of required confirmations. - function MultiSigWallet(address[] _owners, uint _required) - public - { - constructor(_owners, _required); - } - - /// @dev Contract constructor sets initial owners and required number of confirmations, can only be called once. - /// @param _owners List of initial owners. - /// @param _required Number of required confirmations. - function constructor(address[] _owners, uint _required) - public - validRequirement(_owners.length, _required) - { - require(owners.length == 0 && required == 0); - for (uint i = 0; i < _owners.length; i++) { - require(!isOwner[_owners[i]] && _owners[i] != 0); - isOwner[_owners[i]] = true; - } - owners = _owners; - required = _required; - } - - /// @dev Allows to add a new owner. Transaction has to be sent by wallet. - /// @param owner Address of new owner. - function addOwner(address owner) - public - onlyWallet - ownerDoesNotExist(owner) - notNull(owner) - validRequirement(owners.length + 1, required) - { - isOwner[owner] = true; - owners.push(owner); - OwnerAddition(owner); - } - - /// @dev Allows to remove an owner. Transaction has to be sent by wallet. - /// @param owner Address of owner. - function removeOwner(address owner) - public - onlyWallet - ownerExists(owner) - { - isOwner[owner] = false; - for (uint i=0; i owners.length) - changeRequirement(owners.length); - OwnerRemoval(owner); - } - - /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet. - /// @param owner Address of owner to be replaced. - /// @param newOwner Address of new owner. - function replaceOwner(address owner, address newOwner) - public - onlyWallet - ownerExists(owner) - ownerDoesNotExist(newOwner) - { - for (uint i=0; i Date: Fri, 10 Nov 2017 11:07:14 +0200 Subject: [PATCH 24/34] Make activity view linkable, fix landing page link Fixes: #168 --- resources/templates/index.html | 2 +- src/cljs/commiteth/core.cljs | 4 ++++ static_langing_page/index.html | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/resources/templates/index.html b/resources/templates/index.html index 95de5d5..6dfa10c 100644 --- a/resources/templates/index.html +++ b/resources/templates/index.html @@ -77,7 +77,7 @@
- Activity + Activity Open Bounties Log in →
diff --git a/src/cljs/commiteth/core.cljs b/src/cljs/commiteth/core.cljs index f8550dc..80eb9ee 100644 --- a/src/cljs/commiteth/core.cljs +++ b/src/cljs/commiteth/core.cljs @@ -223,6 +223,10 @@ (secretary/defroute "/" [] (rf/dispatch [:set-active-page :bounties])) +(secretary/defroute "/activity" [] + (rf/dispatch [:set-active-page :activity])) + + (secretary/defroute "/repos" [] (if js/user (rf/dispatch [:set-active-page :repos]) diff --git a/static_langing_page/index.html b/static_langing_page/index.html index 95de5d5..6dfa10c 100644 --- a/static_langing_page/index.html +++ b/static_langing_page/index.html @@ -77,7 +77,7 @@
- Activity + Activity Open Bounties Log in →
From 1f255481114c1e07c195327ed811890025f76364 Mon Sep 17 00:00:00 2001 From: Teemu Patja Date: Fri, 10 Nov 2017 12:22:58 +0200 Subject: [PATCH 25/34] Only run web3j for required contracts --- build_contracts.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build_contracts.sh b/build_contracts.sh index 1c5b503..7b00d8c 100755 --- a/build_contracts.sh +++ b/build_contracts.sh @@ -3,8 +3,10 @@ SOLC=$(which solc) WEB3J=$(which web3j) +rm -f resources/contracts/*.{abi,bin} + # compile contracts -for f in contracts/*.sol; do +for f in contracts/{TokenReg,MultiSigTokenWallet*}.sol; do $SOLC $f --overwrite --bin --abi --optimize -o resources/contracts done From e9bccefbb5fc85dc5128b662a366ba59010d2fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 16:20:54 +0000 Subject: [PATCH 26/34] Handle repo installations for Github App --- src/clj/commiteth/routes/services.clj | 5 +- src/clj/commiteth/routes/webhooks.clj | 124 ++++++++++++++++++-------- 2 files changed, 89 insertions(+), 40 deletions(-) diff --git a/src/clj/commiteth/routes/services.clj b/src/clj/commiteth/routes/services.clj index 47a9fec..82c5633 100644 --- a/src/clj/commiteth/routes/services.clj +++ b/src/clj/commiteth/routes/services.clj @@ -68,10 +68,9 @@ :state 0 :hook_id nil})) -;; TODO(oskarth): We want to support these params via webhooks.clj/webhook-app route +;; NOTE(oskarth): This and above two functions about to be deprecated with Github App (defn handle-toggle-repo [user params can-create?] - (log/debug "handle-toggle-repo" user params) - (log/info "MANUAL ACTION REQUIRED: Possibly add repo:" (pr-str user) (pr-str params)) + (log/info "XXX handle-toggle-repo" (pr-str user) (pr-str params)) (let [{user-id :id} user {repo-id :id full-repo :full_name diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 2d734c7..2dcbc81 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -1,20 +1,26 @@ (ns commiteth.routes.webhooks - (:require [cheshire.core :as json] - [clojure.string :as str :refer [join]] - [clojure.tools.logging :as log] - [commiteth.bounties :as bounties] - [commiteth.db - [issues :as issues] - [pull-requests :as pull-requests] - [repositories :as repos] - [users :as users]] - [commiteth.github.core :as github] - [commiteth.util.digest :refer [hex-hmac-sha1]] - [compojure.core :refer [defroutes POST]] - [crypto.equality :as crypto] - [ring.util.http-response :refer [ok forbidden]] - [commiteth.db.bounties :as bounties-db] - [clojure.string :as string]) + (:require + ;; TODO(oskarth): Don't use :refer :all (not sure where services.clj functions come from so importing both. + [ring.util.http-response :refer :all] + [compojure.api.sweet :refer :all] + [cheshire.core :as json] + [clojure.string :as str :refer [join]] + [clojure.tools.logging :as log] + [commiteth.bounties :as bounties] + [commiteth.db + [issues :as issues] + [pull-requests :as pull-requests] + [repositories :as repositories] + [users :as users]] + [commiteth.github.core :as github] + [commiteth.util.digest :refer [hex-hmac-sha1]] + ;; TODO(oskarth): Bad form, put whitelist in better namespace + [commiteth.routes.services :refer [user-whitelisted?]] + [compojure.core :refer [defroutes POST]] + [crypto.equality :as crypto] + [ring.util.http-response :refer [ok forbidden]] + [commiteth.db.bounties :as bounties-db] + [clojure.string :as string]) (:import java.lang.Integer)) (defn find-issue-event @@ -218,31 +224,75 @@ (handle-issue-reopened webhook-payload))) (ok)) -(defn handle-installation [{:keys [action installation sender]}] - ;; TODO(oskarth): Handle other installs? Like remove, say. +(defn enable-repo-2 [repo-id full-repo] + (log/debug "enable-repo-2" repo-id full-repo) + ;; TODO(oskarth): Add granular permissions to enable creation of label + #_(github/create-label full-repo) + (repositories/update-repo repo-id {:state 2}) + (when (add-bounties-for-existing-issues?) + (bounties/add-bounties-for-existing-issues full-repo))) + +(defn disable-repo-2 [repo-id full-repo] + (log/debug "disable-repo-2" repo-id full-repo) + (repositories/update-repo repo-id {:state 0})) + +;; NOTE(oskarth): Together with {enable,disable}-repo-2 above, this replaces +;; handle-toggle-repo for Github App. +(defn handle-add-repo [user-id username owner-avatar-url repo can-create?] + (let [repo-id (:id repo) + repo (:name repo) + full-repo (:full_name repo) + [owner _] (str/split full-repo #"/") + db-user (users/get-user user-id)] + (log/info "handle-add-repo" + (pr-str {:user-id user-id + :name username + :owner-avatar-url owner-avatar-url + :repo-id repo-id + :repo repo + :full-repo full-repo})) + (cond (not can-create?) + {:status 400 + :body "Please join our Riot - chat.status.im/#/register and request + access in our #openbounty room to have your account whitelisted"} + + (empty? (:address db-user)) + {:status 400 + :body "Please add your ethereum address to your profile first"} + + :else + (try + (let [_ (log/info "handle-add-repo pre-create") + db-item (repositories/create (merge params {:user_id user-id + :owner owner})) + is-enabled (= 2 (:state db-item))] + (if is-enabled + (disable-repo-2 repo-id full-repo) + (enable-repo-2 repo-id full-repo)) + (ok (merge + {:enabled (not is-enabled)} + (select-keys params [:id :full_name])))) + (catch Exception e + (log/error "exception when enabling repo" e) + (repositories/update-repo repo-id {:state -1}) + (internal-server-error)))))) + +(defn handle-installation [{:keys [action repositories sender]}] + ;; TODO(oskarth): Handle other installs, like disable. (when (= action "created") (let [user-id (:id sender) username (:login sender) owner-avatar-url (:avatar_url sender) - ;; [{id,name,full_name},...] - repos (:repositories installation) - ;; TODO(oskarth): Handle install for each repo - first-repo (first repos) - ] - ;; NOTE(oskarth): All needed params to call equivalent - ;; handle-toggle-repo. - ;; DON'T need token cause we don't modify webhooks. + first-repo (first repositories) + can-create? (user-whitelisted? (:username username))] (log/info "handle-installation created" - (pr-str - {:user-id user-id - :name username - :owner-avatar-url owner-avatar-url - :all-selected-repos repos - :first-repo-id (:id first-repo) - :first-repo (:name first-repo) - :first-full-name (:full_name first-repo)})) - ;; TODO(oskarth): Actually install repo - ))) + (pr-str {:user-id user-id + :name username + :owner-avatar-url owner-avatar-url + :repos repositories})) + (doseq [repo repositories] + (handle-add-repo user-id username owner-avatar-url repo can-create?)))) + (ok)) (defn handle-pull-request [pull-request] @@ -257,7 +307,7 @@ (defn validate-secret [webhook-payload raw-payload github-signature] ;; used for oauth app webhooks. secret is repo-specific (let [full-name (get-in webhook-payload [:repository :full_name]) - repo (repos/get-repo full-name) + repo (repositories/get-repo full-name) secret (:hook_secret repo)] (and (not (string/blank? secret)) (crypto/eq? github-signature From 1ddb3b70f5be76ba1cf27a8bdf9e8b68f3bb8172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 16:23:25 +0000 Subject: [PATCH 27/34] Fix misc build errors --- src/clj/commiteth/routes/webhooks.clj | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 2dcbc81..762cab2 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -1,8 +1,6 @@ (ns commiteth.routes.webhooks (:require - ;; TODO(oskarth): Don't use :refer :all (not sure where services.clj functions come from so importing both. - [ring.util.http-response :refer :all] - [compojure.api.sweet :refer :all] + [ring.util.http-response :refer [internal-server-error]] [cheshire.core :as json] [clojure.string :as str :refer [join]] [clojure.tools.logging :as log] @@ -14,8 +12,10 @@ [users :as users]] [commiteth.github.core :as github] [commiteth.util.digest :refer [hex-hmac-sha1]] - ;; TODO(oskarth): Bad form, put whitelist in better namespace - [commiteth.routes.services :refer [user-whitelisted?]] + ;; TODO(oskarth): Bad form, put these in better namespace + [commiteth.routes.services :refer + [user-whitelisted? + add-bounties-for-existing-issues?]] [compojure.core :refer [defroutes POST]] [crypto.equality :as crypto] [ring.util.http-response :refer [ok forbidden]] @@ -263,15 +263,20 @@ :else (try (let [_ (log/info "handle-add-repo pre-create") - db-item (repositories/create (merge params {:user_id user-id - :owner owner})) + db-item (repositories/create + {:id repo-id ;; XXX: Being rename twice... silly. + :name repo ;; XXX: Is this name of repo? + :owner-avatar-url owner-avatar-url + :user_id user-id + :owner owner}) + _ (log/info "handle-add-repo db-item" db-item) is-enabled (= 2 (:state db-item))] (if is-enabled (disable-repo-2 repo-id full-repo) (enable-repo-2 repo-id full-repo)) - (ok (merge - {:enabled (not is-enabled)} - (select-keys params [:id :full_name])))) + (ok {:enabled (not is-enabled) + :id repo-id + :full_name full-repo})) (catch Exception e (log/error "exception when enabling repo" e) (repositories/update-repo repo-id {:state -1}) From 4afec1da1e0a6716a22bf5f3e9db49097d275cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 17:09:31 +0000 Subject: [PATCH 28/34] Parse full repo into owner in try-catch --- src/clj/commiteth/routes/webhooks.clj | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 762cab2..5b835b9 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -236,13 +236,23 @@ (log/debug "disable-repo-2" repo-id full-repo) (repositories/update-repo repo-id {:state 0})) +(defn full-repo->owner [full-repo] + (try + (let [[owner _] (str/split full-repo #"/")] + owner) + (catch Exception e + (log/error "exception when parsing repo" e) + nil))) + ;; NOTE(oskarth): Together with {enable,disable}-repo-2 above, this replaces ;; handle-toggle-repo for Github App. (defn handle-add-repo [user-id username owner-avatar-url repo can-create?] (let [repo-id (:id repo) repo (:name repo) full-repo (:full_name repo) - [owner _] (str/split full-repo #"/") + _ (log/info "handle-installation add pre repo" repo) + owner (full-repo->owner full-repo) + _ (log/info "handle-installation add" full-repo " " owner) db-user (users/get-user user-id)] (log/info "handle-add-repo" (pr-str {:user-id user-id @@ -296,6 +306,7 @@ :owner-avatar-url owner-avatar-url :repos repositories})) (doseq [repo repositories] + (log/info "handle-installation add pre repo" repo) (handle-add-repo user-id username owner-avatar-url repo can-create?)))) (ok)) From 9aec3dba3b025518319a7ecfd1b462929799e400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 20:48:24 +0000 Subject: [PATCH 29/34] Support handle-installation-integration webhook & deprecated callers --- src/clj/commiteth/routes/webhooks.clj | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 5b835b9..9b4f6be 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -310,6 +310,27 @@ (handle-add-repo user-id username owner-avatar-url repo can-create?)))) (ok)) +(defn handle-installation-integration [{:keys [action sender] :as payload}] + ;; TODO(oskarth): Handle other installs, like disable. + ;; TODO(oskarth): Also support remove in :repositories_removed + ;; TODO(oskarth): Also support case when :repository_selection is all - does it work differently? + (when (= action "added") + (let [repositories (:repositories_added payload) + user-id (:id sender) + username (:login sender) + owner-avatar-url (:avatar_url sender) + first-repo (first repositories) + can-create? (user-whitelisted? (:username username))] + (log/info "handle-installation-integration created" + (pr-str {:user-id user-id + :name username + :owner-avatar-url owner-avatar-url + :repos repositories})) + (doseq [repo repositories] + (log/info "handle-installation-integration add pre repo" repo) + (handle-add-repo user-id username owner-avatar-url repo can-create?)))) + (ok)) + (defn handle-pull-request [pull-request] (let [action (keyword (:action pull-request))] @@ -368,6 +389,14 @@ "issues" (handle-issue payload) "pull_request" (handle-pull-request payload) "installation" (handle-installation payload) + "installation_repositories" (handle-installation-repositories payload) + + ;; NOTE(oskarth): These two webhooks are / will be deprecated on + ;; November 22, 2017 but they keep being called. According to + ;; documentation they should contain same format. + ;; https://developer.github.com/webhooks/ + "integration_installation" (handle-installation payload) + "integration_installation_repositories" (handle-installation-repositories payload) (ok))) (forbidden))))) From afcbfa0e4b2d44e8149202866e9a3b4238e92161 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 20:54:15 +0000 Subject: [PATCH 30/34] Fix symbol name --- src/clj/commiteth/routes/webhooks.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 9b4f6be..977e065 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -310,7 +310,7 @@ (handle-add-repo user-id username owner-avatar-url repo can-create?)))) (ok)) -(defn handle-installation-integration [{:keys [action sender] :as payload}] +(defn handle-installation-repositories [{:keys [action sender] :as payload}] ;; TODO(oskarth): Handle other installs, like disable. ;; TODO(oskarth): Also support remove in :repositories_removed ;; TODO(oskarth): Also support case when :repository_selection is all - does it work differently? From cc05f36e29aa51bafed58c5bfd909c6d51f6ae14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 21:19:12 +0000 Subject: [PATCH 31/34] Fix shadow repo variable bug --- src/clj/commiteth/routes/webhooks.clj | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 977e065..83dd780 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -248,9 +248,9 @@ ;; handle-toggle-repo for Github App. (defn handle-add-repo [user-id username owner-avatar-url repo can-create?] (let [repo-id (:id repo) - repo (:name repo) + repo-name (:name repo) full-repo (:full_name repo) - _ (log/info "handle-installation add pre repo" repo) + _ (log/info "handle-installation add pre repo" (pr-str repo) " " (pr-str full-repo)) owner (full-repo->owner full-repo) _ (log/info "handle-installation add" full-repo " " owner) db-user (users/get-user user-id)] @@ -259,7 +259,7 @@ :name username :owner-avatar-url owner-avatar-url :repo-id repo-id - :repo repo + :repo repo-name :full-repo full-repo})) (cond (not can-create?) {:status 400 From 764f60edcbc9cefe30e4379df7b939e033c2cc2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 21:32:38 +0000 Subject: [PATCH 32/34] Fix can-create? bug and log it --- src/clj/commiteth/routes/webhooks.clj | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 83dd780..4ab2c42 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -260,7 +260,8 @@ :owner-avatar-url owner-avatar-url :repo-id repo-id :repo repo-name - :full-repo full-repo})) + :full-repo full-repo + :can-create? can-create?})) (cond (not can-create?) {:status 400 :body "Please join our Riot - chat.status.im/#/register and request @@ -299,7 +300,7 @@ username (:login sender) owner-avatar-url (:avatar_url sender) first-repo (first repositories) - can-create? (user-whitelisted? (:username username))] + can-create? (user-whitelisted? username)] (log/info "handle-installation created" (pr-str {:user-id user-id :name username @@ -320,7 +321,7 @@ username (:login sender) owner-avatar-url (:avatar_url sender) first-repo (first repositories) - can-create? (user-whitelisted? (:username username))] + can-create? (user-whitelisted? username)] (log/info "handle-installation-integration created" (pr-str {:user-id user-id :name username From eb8edec1166bcaccfbbcff28c3aa9e0ccf5c7f59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 21:37:53 +0000 Subject: [PATCH 33/34] Add repo-name to DB; better logging for failed repo add --- src/clj/commiteth/routes/webhooks.clj | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/clj/commiteth/routes/webhooks.clj b/src/clj/commiteth/routes/webhooks.clj index 4ab2c42..482c4bd 100644 --- a/src/clj/commiteth/routes/webhooks.clj +++ b/src/clj/commiteth/routes/webhooks.clj @@ -263,20 +263,22 @@ :full-repo full-repo :can-create? can-create?})) (cond (not can-create?) - {:status 400 - :body "Please join our Riot - chat.status.im/#/register and request - access in our #openbounty room to have your account whitelisted"} + (do (log/info "handle-add-repo user not in whitelist: " username) + {:status 400 + :body "Please join our Riot - chat.status.im/#/register and request + access in our #openbounty room to have your account whitelisted"}) (empty? (:address db-user)) - {:status 400 - :body "Please add your ethereum address to your profile first"} + (do (log/info "handle-add-repo user lacking ethereum address: " (pr-str db-user)) + {:status 400 + :body "Please add your ethereum address to your profile first"}) :else (try (let [_ (log/info "handle-add-repo pre-create") db-item (repositories/create {:id repo-id ;; XXX: Being rename twice... silly. - :name repo ;; XXX: Is this name of repo? + :name repo-name ;; XXX: Is this name of repo? :owner-avatar-url owner-avatar-url :user_id user-id :owner owner}) From 1db018912b6c6fa690e6a2a5765945ae27f39956 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Fri, 10 Nov 2017 21:48:38 +0000 Subject: [PATCH 34/34] Disable repositories tab --- src/cljs/commiteth/core.cljs | 4 +++- test/cljs/commiteth/cards.cljs | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cljs/commiteth/core.cljs b/src/cljs/commiteth/core.cljs index 80eb9ee..8a90933 100644 --- a/src/cljs/commiteth/core.cljs +++ b/src/cljs/commiteth/core.cljs @@ -79,7 +79,9 @@ (let [tabs (apply conj [[:bounties (str (when-not @user "Open ") "Bounties")] [:activity "Activity"]] (when @user - [[:repos "Repositories"] + [ + ;; NOTE(oskarth) Disabling this as repo management happens through GH app + #_[:repos "Repositories"] [:manage-payouts (str (when-not mobile? "Manage ") "Payouts")] (when (:status-team-member? @user) [:usage-metrics "Usage metrics"])]))] diff --git a/test/cljs/commiteth/cards.cljs b/test/cljs/commiteth/cards.cljs index a22ebe8..fa240c5 100644 --- a/test/cljs/commiteth/cards.cljs +++ b/test/cljs/commiteth/cards.cljs @@ -106,7 +106,8 @@ (let [active-tab (:active-tab app-state)] [:div.ui.attached.tabular.menu.tiny (for [[tab caption] [[:activity "Activity"] - [:manage "Repositories"] + ;; NOTE(oskarth) Disabling this as repo management happens through GH app + #_[:manage "Repositories"] [:bounties "Bounties"]]] (let [props {:class (str "ui item" (when (= active-tab tab) " active"))