diff --git a/packages/core/contracts/Escrow.sol b/packages/core/contracts/Escrow.sol index be82de10cf..d2c14a1911 100644 --- a/packages/core/contracts/Escrow.sol +++ b/packages/core/contracts/Escrow.sol @@ -4,12 +4,13 @@ pragma solidity >=0.6.2; import '@openzeppelin/contracts/token/ERC20/IERC20.sol'; import '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol'; +import '@openzeppelin/contracts/security/ReentrancyGuard.sol'; import './interfaces/IRewardPool.sol'; import './interfaces/IEscrow.sol'; import './utils/SafeMath.sol'; -contract Escrow is IEscrow { +contract Escrow is IEscrow, ReentrancyGuard { using SafeMath for uint256; bytes4 private constant FUNC_SELECTOR_BALANCE_OF = @@ -17,9 +18,14 @@ contract Escrow is IEscrow { bytes4 private constant FUNC_SELECTOR_TRANSFER = bytes4(keccak256('transfer(address,uint256)')); + string constant ERROR_ZERO_ADDRESS = 'Escrow: zero address'; + + event TrustedHandlerAdded(address _handler); event IntermediateStorage(string _url, string _hash); event Pending(string manifest, string hash); event BulkTransfer(uint256 indexed _txId, uint256 _bulkCount); + event Cancelled(); + event Completed(); EscrowStatuses public override status; @@ -30,7 +36,7 @@ contract Escrow is IEscrow { uint256 public reputationOracleStake; uint256 public recordingOracleStake; - uint256 private constant BULK_MAX_VALUE = 1000000000 * (10 ** 18); + uint256 private constant BULK_MAX_VALUE = 1e9 * (10 ** 18); uint32 private constant BULK_MAX_COUNT = 100; address public token; @@ -55,6 +61,9 @@ contract Escrow is IEscrow { uint256 _duration, address[] memory _handlers ) { + require(_token != address(0), ERROR_ZERO_ADDRESS); + require(_canceler != address(0), ERROR_ZERO_ADDRESS); + token = _token; status = EscrowStatuses.Launched; duration = _duration.add(block.timestamp); // solhint-disable-line not-rely-on-time @@ -81,7 +90,9 @@ contract Escrow is IEscrow { 'Address calling cannot add trusted handlers' ); for (uint256 i = 0; i < _handlers.length; i++) { + require(_handlers[i] != address(0), ERROR_ZERO_ADDRESS); areTrustedHandlers[_handlers[i]] = true; + emit TrustedHandlerAdded(_handlers[i]); } } @@ -96,7 +107,7 @@ contract Escrow is IEscrow { string memory _url, string memory _hash, uint256 _solutionsRequested - ) public trusted notExpired { + ) external trusted notExpired { require( _reputationOracle != address(0), 'Invalid or missing token spender' @@ -107,7 +118,7 @@ contract Escrow is IEscrow { ); require(_solutionsRequested > 0, 'Invalid or missing solutions'); uint256 totalStake = _reputationOracleStake.add(_recordingOracleStake); - require(totalStake >= 0 && totalStake <= 100, 'Stake out of bounds'); + require(totalStake <= 100, 'Stake out of bounds'); require( status == EscrowStatuses.Launched, 'Escrow not in Launched status state' @@ -128,7 +139,7 @@ contract Escrow is IEscrow { emit Pending(manifestUrl, manifestHash); } - function abort() public trusted notComplete notPaid { + function abort() external trusted notComplete notPaid { if (getBalance() != 0) { cancel(); } @@ -141,31 +152,35 @@ contract Escrow is IEscrow { notBroke notComplete notPaid + nonReentrant returns (bool) { _safeTransfer(canceler, getBalance()); status = EscrowStatuses.Cancelled; + emit Cancelled(); return true; } - function complete() public notExpired { + function complete() external notExpired { require( - msg.sender == reputationOracle || areTrustedHandlers[msg.sender], + areTrustedHandlers[msg.sender], 'Address calling is not trusted' ); require(status == EscrowStatuses.Paid, 'Escrow not in Paid state'); status = EscrowStatuses.Complete; + emit Completed(); } function storeResults( string memory _url, string memory _hash - ) public trusted notExpired { + ) external trusted notExpired { require( status == EscrowStatuses.Pending || status == EscrowStatuses.Partial, 'Escrow not in Pending or Partial status state' ); + _storeResult(_url, _hash); emit IntermediateStorage(_url, _hash); } @@ -175,18 +190,32 @@ contract Escrow is IEscrow { string memory _url, string memory _hash, uint256 _txId - ) public trusted notBroke notLaunched notPaid notExpired returns (bool) { + ) + external + trusted + notBroke + notLaunched + notPaid + notExpired + nonReentrant + returns (bool) + { require( _recipients.length == _amounts.length, "Amount of recipients and values don't match" ); require(_recipients.length < BULK_MAX_COUNT, 'Too many recipients'); + require( + status != EscrowStatuses.Complete && + status != EscrowStatuses.Cancelled, + 'Invalid status' + ); uint256 balance = getBalance(); bulkPaid = false; uint256 aggregatedBulkAmount = 0; for (uint256 i; i < _amounts.length; i++) { - aggregatedBulkAmount += _amounts[i]; + aggregatedBulkAmount = aggregatedBulkAmount.add(_amounts[i]); } require(aggregatedBulkAmount < BULK_MAX_VALUE, 'Bulk value too high'); @@ -194,12 +223,7 @@ contract Escrow is IEscrow { return bulkPaid; } - bool writeOnchain = bytes(_hash).length != 0 || bytes(_url).length != 0; - if (writeOnchain) { - // Be sure they are both zero if one of them is - finalResultsUrl = _url; - finalResultsHash = _hash; - } + _storeResult(_url, _hash); ( uint256 reputationOracleFee, @@ -207,7 +231,12 @@ contract Escrow is IEscrow { ) = finalizePayouts(_amounts); for (uint256 i = 0; i < _recipients.length; ++i) { - _safeTransfer(_recipients[i], finalAmounts[i]); + uint256 amount = finalAmounts[i]; + if (amount == 0) { + continue; + } + finalAmounts[i] = 0; + _safeTransfer(_recipients[i], amount); } delete finalAmounts; @@ -268,6 +297,15 @@ contract Escrow is IEscrow { SafeERC20.safeTransfer(IERC20(token), to, value); } + function _storeResult(string memory _url, string memory _hash) internal { + bool writeOnchain = bytes(_hash).length != 0 || bytes(_url).length != 0; + if (writeOnchain) { + // Be sure both of them are not zero + finalResultsUrl = _url; + finalResultsHash = _hash; + } + } + modifier trusted() { require(areTrustedHandlers[msg.sender], 'Address calling not trusted'); _; diff --git a/packages/core/contracts/HMToken.sol b/packages/core/contracts/HMToken.sol index 598e9a4fe1..25a3abbdcf 100644 --- a/packages/core/contracts/HMToken.sol +++ b/packages/core/contracts/HMToken.sol @@ -52,7 +52,7 @@ contract HMToken is HMTokenInterface, Ownable { function transfer( address _to, uint256 _value - ) public override returns (bool success) { + ) external override returns (bool success) { success = transferQuiet(_to, _value); require(success, "Transfer didn't succeed"); return success; @@ -62,7 +62,11 @@ contract HMToken is HMTokenInterface, Ownable { address _spender, address _to, uint256 _value - ) public override returns (bool success) { + ) external override returns (bool success) { + require( + _spender != address(0), + "Can't send tokens to uninitialized address" + ); uint256 _allowance = allowed[_spender][msg.sender]; require(_allowance >= _value, 'Spender allowance too low'); require( @@ -89,14 +93,14 @@ contract HMToken is HMTokenInterface, Ownable { function balanceOf( address _owner - ) public view override returns (uint256 balance) { + ) external view override returns (uint256 balance) { return balances[_owner]; } function approve( address _spender, uint256 _value - ) public override returns (bool success) { + ) external override returns (bool success) { require( _spender != address(0), 'Token spender is an uninitialized address' @@ -118,8 +122,7 @@ contract HMToken is HMTokenInterface, Ownable { uint256 _oldValue = allowed[msg.sender][_spender]; if ( - _oldValue.add(_delta) < _oldValue || - _oldValue.add(_delta) >= MAX_UINT256 + _oldValue + _delta < _oldValue || _oldValue + _delta == MAX_UINT256 ) { // Truncate upon overflow. allowed[msg.sender][_spender] = MAX_UINT256.sub(1); @@ -135,7 +138,7 @@ contract HMToken is HMTokenInterface, Ownable { function decreaseApproval( address _spender, uint256 _delta - ) public returns (bool success) { + ) external returns (bool success) { require( _spender != address(0), 'Token spender is an uninitialized address' @@ -157,7 +160,7 @@ contract HMToken is HMTokenInterface, Ownable { function allowance( address _owner, address _spender - ) public view override returns (uint256 remaining) { + ) external view override returns (uint256 remaining) { return allowed[_owner][_spender]; } @@ -165,7 +168,7 @@ contract HMToken is HMTokenInterface, Ownable { address[] memory _tos, uint256[] memory _values, uint256 _txId - ) public override returns (uint256 _bulkCount) { + ) external override returns (uint256 _bulkCount) { require( _tos.length == _values.length, "Amount of recipients and values don't match" @@ -189,11 +192,11 @@ contract HMToken is HMTokenInterface, Ownable { return _bulkCount; } - function approveBulk( + function increaseApprovalBulk( address[] memory _spenders, uint256[] memory _values, uint256 _txId - ) public returns (uint256 _bulkCount) { + ) external returns (uint256 _bulkCount) { require( _spenders.length == _values.length, "Amount of spenders and values don't match" @@ -222,9 +225,11 @@ contract HMToken is HMTokenInterface, Ownable { address _to, uint256 _value ) internal returns (bool success) { - if (_to == address(0)) return false; // Preclude burning tokens to uninitialized address. - if (_to == address(this)) return false; // Preclude sending tokens to the contract. - if (balances[msg.sender] < _value) return false; + if ( + _to == address(0) || + _to == address(this) || + balances[msg.sender] < _value + ) return false; // Preclude burning tokens to uninitialized address, or sending tokens to the contract. balances[msg.sender] = balances[msg.sender] - _value; balances[_to] = balances[_to].add(_value); diff --git a/packages/core/contracts/RewardPool.sol b/packages/core/contracts/RewardPool.sol index f58f1331ab..3bfb9fc3fc 100644 --- a/packages/core/contracts/RewardPool.sol +++ b/packages/core/contracts/RewardPool.sol @@ -2,8 +2,8 @@ pragma solidity >=0.6.2; -import '@openzeppelin/contracts/token/ERC20/IERC20.sol'; -import '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol'; +import '@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol'; +import '@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol'; import '@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol'; import '@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol'; @@ -118,7 +118,7 @@ contract RewardPool is IRewardPool, OwnableUpgradeable, UUPSUpgradeable { } function _safeTransfer(address to, uint256 value) internal { - SafeERC20.safeTransfer(IERC20(token), to, value); + SafeERC20Upgradeable.safeTransfer(IERC20Upgradeable(token), to, value); } modifier onlyStaking() { diff --git a/packages/core/contracts/Staking.sol b/packages/core/contracts/Staking.sol index b8092c67c5..f6658dfe81 100644 --- a/packages/core/contracts/Staking.sol +++ b/packages/core/contracts/Staking.sol @@ -2,8 +2,8 @@ pragma solidity >=0.6.2; -import '@openzeppelin/contracts/token/ERC20/IERC20.sol'; -import '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol'; +import '@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol'; +import '@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol'; import '@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol'; import '@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol'; @@ -548,7 +548,7 @@ contract Staking is IStaking, OwnableUpgradeable, UUPSUpgradeable { } function _safeTransfer(address to, uint256 value) internal { - SafeERC20.safeTransfer(IERC20(token), to, value); + SafeERC20Upgradeable.safeTransfer(IERC20Upgradeable(token), to, value); } function _safeTransferFrom( @@ -556,7 +556,12 @@ contract Staking is IStaking, OwnableUpgradeable, UUPSUpgradeable { address to, uint256 value ) internal { - SafeERC20.safeTransferFrom(IERC20(token), from, to, value); + SafeERC20Upgradeable.safeTransferFrom( + IERC20Upgradeable(token), + from, + to, + value + ); } modifier onlyStaker(address _staker) { diff --git a/packages/core/package.json b/packages/core/package.json index e869decffd..66c358434b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,7 +1,7 @@ { "name": "@human-protocol/core", "description": "Human Protocol Core Smart Contracts", - "version": "1.0.37", + "version": "1.0.39", "files": [ "contracts/**/*.sol", "abis/**/*.json", diff --git a/packages/core/test/EscrowFactory.ts b/packages/core/test/EscrowFactory.ts index e85b6ef57d..492ef3fc2f 100644 --- a/packages/core/test/EscrowFactory.ts +++ b/packages/core/test/EscrowFactory.ts @@ -24,7 +24,9 @@ describe('EscrowFactory', function () { .connect(operator) .createEscrow(token.address, trustedHandlers) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; return event; } @@ -85,7 +87,7 @@ describe('EscrowFactory', function () { await expect( escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await reputationOracle.getAddress()]) ).to.be.revertedWith('Needs to stake HMT tokens to create an escrow.'); }); @@ -141,7 +143,7 @@ describe('EscrowFactory', function () { await expect( escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await reputationOracle.getAddress()]) ).to.be.revertedWith('Needs to stake HMT tokens to create an escrow.'); }); diff --git a/packages/core/test/RewardPool.ts b/packages/core/test/RewardPool.ts index 8ebbf84c83..8bac0c9150 100644 --- a/packages/core/test/RewardPool.ts +++ b/packages/core/test/RewardPool.ts @@ -127,9 +127,11 @@ describe('RewardPool', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal(token.address, 'token address is correct'); expect(event?.escrow).to.not.be.null; @@ -213,9 +215,11 @@ describe('RewardPool', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal(token.address, 'token address is correct'); expect(event?.escrow).to.not.be.null; diff --git a/packages/core/test/Staking.ts b/packages/core/test/Staking.ts index f4059b14ec..73ae0f89d5 100644 --- a/packages/core/test/Staking.ts +++ b/packages/core/test/Staking.ts @@ -231,9 +231,11 @@ describe('Staking', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal(token.address, 'token address is correct'); expect(event?.escrow).to.not.be.null; @@ -327,9 +329,11 @@ describe('Staking', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal(token.address, 'token address is correct'); expect(event?.escrow).to.not.be.null; @@ -517,9 +521,11 @@ describe('Staking', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal( token.address, @@ -581,9 +587,11 @@ describe('Staking', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal( token.address, @@ -638,9 +646,11 @@ describe('Staking', function () { const result = await ( await escrowFactory .connect(operator) - .createEscrow(token.address, [ethers.constants.AddressZero]) + .createEscrow(token.address, [await validator.getAddress()]) ).wait(); - const event = result.events?.[0].args; + const event = result.events?.find(({ topics }) => + topics.includes(ethers.utils.id('Launched(address,address)')) + )?.args; expect(event?.token).to.equal(token.address, 'token address is correct'); expect(event?.escrow).to.not.be.null; diff --git a/packages/sdk/typescript/human-protocol-sdk/package.json b/packages/sdk/typescript/human-protocol-sdk/package.json index 13794e6b68..fb2e1b4c96 100644 --- a/packages/sdk/typescript/human-protocol-sdk/package.json +++ b/packages/sdk/typescript/human-protocol-sdk/package.json @@ -14,7 +14,7 @@ "clean": "rm -rf ./dist", "build": "npm run clean && tsc", "prepublish": "npm run build", - "test": "concurrently -k -s first -g --hide 0 \"yarn workspace @human-protocol/core local\" \"sleep 5 && jest --runInBand\"", + "test": "concurrently -k -s first -g --hide 0 \"yarn workspace @human-protocol/core local\" \"sleep 10 && jest --runInBand\"", "lint": "eslint .", "lint:fix": "eslint . --fix", "format": "prettier --write '**/*.{ts,json}'" @@ -45,8 +45,5 @@ "ethers": "^5.7.2", "secp256k1": "^4.0.3", "winston": "^3.8.2" - }, - "peerDependencies": { - "@human-protocol/core": "^1.0.12" } } diff --git a/packages/sdk/typescript/subgraph/.eslintignore b/packages/sdk/typescript/subgraph/.eslintignore index 86d4c2dd38..7bbdc897ae 100644 --- a/packages/sdk/typescript/subgraph/.eslintignore +++ b/packages/sdk/typescript/subgraph/.eslintignore @@ -1 +1,2 @@ generated +schema.graphql