Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix audit report #194

Merged
merged 8 commits into from
Jan 31, 2023
Merged

Fix audit report #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b49dd5
fix audit report
leric7 Jan 25, 2023
06328cc
fix oz contract import
leric7 Jan 25, 2023
7a0cc30
fix core test failing
leric7 Jan 25, 2023
61a0697
update core package version
leric7 Jan 25, 2023
f1d3525
fix eslint
leric7 Jan 25, 2023
fca156f
fix jest cli version
leric7 Jan 26, 2023
8833b2c
fix dependency
leric7 Jan 26, 2023
61ff9a7
revert yarn.lock
leric7 Jan 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 55 additions & 17 deletions packages/core/contracts/Escrow.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,28 @@ 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 =
bytes4(keccak256('balanceOf(address)'));
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;

Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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]);
}
}

Expand All @@ -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'
Expand All @@ -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'
Expand All @@ -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();
}
Expand All @@ -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);
}

Expand All @@ -175,39 +190,53 @@ 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');

if (balance < aggregatedBulkAmount) {
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,
uint256 recordingOracleFee
) = 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;
Expand Down Expand Up @@ -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');
_;
Expand Down
33 changes: 19 additions & 14 deletions packages/core/contracts/HMToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -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'
Expand All @@ -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);
Expand All @@ -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'
Expand All @@ -157,15 +160,15 @@ 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];
}

function transferBulk(
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"
Expand All @@ -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"
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions packages/core/contracts/RewardPool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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() {
Expand Down
13 changes: 9 additions & 4 deletions packages/core/contracts/Staking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -548,15 +548,20 @@ 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(
address from,
address to,
uint256 value
) internal {
SafeERC20.safeTransferFrom(IERC20(token), from, to, value);
SafeERC20Upgradeable.safeTransferFrom(
IERC20Upgradeable(token),
from,
to,
value
);
}

modifier onlyStaker(address _staker) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
Loading