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.

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

Fix/audit results #68

Merged
merged 11 commits into from
Sep 5, 2024
2 changes: 1 addition & 1 deletion contracts/CrossChainProofOfHumanity.sol
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ contract CrossChainProofOfHumanity is ICrossChainProofOfHumanity {
return !humanity.isHomeChain
&& humanityId != bytes20(0x0)
&& humanity.owner == _account
&& humanity.expirationTime > block.timestamp;
&& humanity.expirationTime >= block.timestamp;
}

/** @notice Get the owner of a humanity. Returns null address if not claimed
Expand Down
79 changes: 46 additions & 33 deletions contracts/ProofOfHumanity.sol
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
uint256 public winnerStakeMultiplier;
/// @dev Multiplier for calculating the fee stake paid by the party that lost the previous round.
uint256 public loserStakeMultiplier;
/// @dev The number of humanities registered at some moment.
uint256 public humanityCount;

/// @dev Gap for possible future versions storage layout changes.
uint256[50] internal __gap;
Expand Down Expand Up @@ -799,7 +801,10 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
requestBaseDeposit
);

require(_contribute(_humanityId, requestId, 0, 0, Party.Requester, totalCost));
_contribute(_humanityId, requestId, 0, 0, Party.Requester, totalCost);
Round storage round = request.challenges[0].rounds[0];
require(round.paidFees.forRequester >= totalCost);
round.sideFunded = Party.Requester;

emit RevocationRequest(msg.sender, _humanityId, requestId);
emit Evidence(
Expand Down Expand Up @@ -828,6 +833,10 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
);

_contribute(_humanityId, _requestId, 0, 0, Party.Requester, totalCost);
Round storage round = request.challenges[0].rounds[0];

if (round.paidFees.forRequester >= totalCost)
round.sideFunded = Party.Requester;
}

/** @notice Vouch that the human corresponds to the humanity ID.
Expand Down Expand Up @@ -1032,12 +1041,14 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {

uint256 challengeId = request.lastChallengeId++;
Challenge storage challenge = request.challenges[challengeId];
Round storage round = challenge.rounds[0];

ArbitratorData memory arbitratorData = arbitratorDataHistory[request.arbitratorDataId];
uint256 arbitrationCost = arbitratorData.arbitrator.arbitrationCost(arbitratorData.arbitratorExtraData);

require(_contribute(_humanityId, _requestId, challengeId, 0, Party.Challenger, arbitrationCost));
_contribute(_humanityId, _requestId, challengeId, 0, Party.Challenger, arbitrationCost);
Round storage round = challenge.rounds[0];
require(round.paidFees.forChallenger >= arbitrationCost);
round.sideFunded = Party.None; // Set this back to 0, since it's no longer relevant as the new round is created.

// Subtract the costs from the total of staked contributions.
round.feeRewards = round.feeRewards.subCap(arbitrationCost);
Expand Down Expand Up @@ -1108,37 +1119,32 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
Challenge storage challenge = request.challenges[disputeData.challengeId];
Round storage round = challenge.rounds[challenge.lastRoundId];

Party firstFunded = round.sideFunded;
require(_side != firstFunded);
require(_side != round.sideFunded);

uint256 appealCost = IArbitrator(_arbitrator).appealCost(
_disputeId,
arbitratorDataHistory[request.arbitratorDataId].arbitratorExtraData
);
uint256 totalCost = appealCost.addCap(appealCost.mulCap(multiplier) / MULTIPLIER_DIVISOR);

if (
_contribute(
disputeData.humanityId,
disputeData.requestId,
disputeData.challengeId,
challenge.lastRoundId,
_side,
totalCost
) &&
_contribute(disputeData.humanityId, disputeData.requestId, disputeData.challengeId, challenge.lastRoundId, _side, totalCost);
uint256 paidFees = _side == Party.Requester ? round.paidFees.forRequester : round.paidFees.forChallenger;
if (paidFees >= totalCost) {
// If firstFunded was assigned, it means other side was funded and if this one gets fully funded as well appeal can be created.
firstFunded != Party.None
) {
IArbitrator(_arbitrator).appeal{value: appealCost}(
_disputeId,
arbitratorDataHistory[request.arbitratorDataId].arbitratorExtraData
);
challenge.lastRoundId++;

// Subtract the costs from the total of staked contributions
round.feeRewards = round.feeRewards.subCap(appealCost);

emit AppealCreated(IArbitrator(_arbitrator), _disputeId);
if (round.sideFunded == Party.None) {
round.sideFunded = _side;
} else {
IArbitrator(_arbitrator).appeal{value: appealCost}(
_disputeId,
arbitratorDataHistory[request.arbitratorDataId].arbitratorExtraData
);
challenge.lastRoundId++;

// Subtract the costs from the total of staked contributions
round.feeRewards = round.feeRewards.subCap(appealCost);
round.sideFunded = Party.None; // Set this back to default in the past round as it's no longer relevant.

emit AppealCreated(IArbitrator(_arbitrator), _disputeId);
}
}
}

Expand Down Expand Up @@ -1410,6 +1416,7 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
Humanity storage humanity = humanityData[_humanityId];

requestId = humanity.requests.length;
if (requestId == 0) humanityCount++;

Request storage request = humanity.requests.push();
request.requester = payable(msg.sender);
Expand All @@ -1425,7 +1432,11 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
uint256 totalCost = arbitratorData.arbitrator.arbitrationCost(arbitratorData.arbitratorExtraData).addCap(
requestBaseDeposit
);

_contribute(_humanityId, requestId, 0, 0, Party.Requester, totalCost);
Round storage round = request.challenges[0].rounds[0];
if (round.paidFees.forRequester >= totalCost)
round.sideFunded = Party.Requester;
}

/** @dev Make a fee contribution. Reimburse remaining ETH.
Expand All @@ -1435,7 +1446,6 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
* @param _roundId Round to contribute to.
* @param _side Side to contribute to.
* @param _totalRequired Total amount required for this side.
* @return paidInFull Whether the contribution was paid in full.
*/
function _contribute(
bytes20 _humanityId,
Expand All @@ -1444,9 +1454,9 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
uint256 _roundId,
Party _side,
uint256 _totalRequired
) internal returns (bool paidInFull) {
) internal {
Round storage round = humanityData[_humanityId].requests[_requestId].challenges[_challengeId].rounds[_roundId];

uint256 remainingETH;
uint256 contribution = msg.value;
uint256 requiredAmount = _totalRequired.subCap(
Expand All @@ -1455,9 +1465,6 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
if (requiredAmount <= msg.value) {
contribution = requiredAmount;
remainingETH = msg.value - requiredAmount;

paidInFull = true;
round.sideFunded = round.sideFunded == Party.None ? _side : Party.None;
}

if (_side == Party.Requester) {
Expand Down Expand Up @@ -1672,4 +1679,10 @@ contract ProofOfHumanity is IProofOfHumanity, IArbitrable, IEvidence {
function getNumberOfVouches(bytes20 _humanityId, uint256 _requestId) external view returns (uint256) {
return humanityData[_humanityId].requests[_requestId].vouches.length;
}

/** @notice Get the number of humanities registered at some moment.
*/
function getHumanityCount() external view returns (uint256) {
return humanityCount;
}
}
102 changes: 102 additions & 0 deletions contracts/ProofOfHumanityProxyV2.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/**
* @authors: [@unknownunknown1*, @clesaege]
* @reviewers: []
* @auditors: []
* @bounties: []
* @deployments: []
* @tools: []
*/

pragma solidity 0.8.20;

import {IProofOfHumanity} from "./interfaces/IProofOfHumanity.sol";

/**
* @title ProofOfHumanityProxyV2
* A proxy contract for ProofOfHumanity that implements a token interface to interact with other dapps.
* Note that it isn't an ERC20 and only implements its interface in order to be compatible with Snapshot.
*/
contract ProofOfHumanityProxyV2 {

// ========== STORAGE ==========

/// @dev The address that can make governance changes to the parameters of the contract.
address public governor;

/// @dev Instance of the ProofOfHumanity contract
IProofOfHumanity public proofOfHumanity;

string public name = "Human Vote";

Check warning

Code scanning / Slither

State variables that could be declared constant Warning

ProofOfHumanityProxyV2.name should be constant
string public symbol = "VOTE";

Check warning

Code scanning / Slither

State variables that could be declared constant Warning

ProofOfHumanityProxyV2.symbol should be constant
uint8 public decimals = 0;

Check warning

Code scanning / Slither

State variables that could be declared constant Warning

ProofOfHumanityProxyV2.decimals should be constant

/* Modifiers */

modifier onlyGovernor() {
require(msg.sender == governor);
_;
}

// ========== CONSTRUCTOR ==========

/** @dev Constructor.
* @param _proofOfHumanity The address of the related ProofOfHumanity contract.
*/
constructor(IProofOfHumanity _proofOfHumanity) {
proofOfHumanity = _proofOfHumanity;
}


/** @dev Changes the address of the the related ProofOfHumanity contract.
* @param _proofOfHumanity The address of the new contract.
*/
function changePoH(IProofOfHumanity _proofOfHumanity) external onlyGovernor {

Check warning

Code scanning / Slither

Conformance to Solidity naming conventions Warning

proofOfHumanity = _proofOfHumanity;
}

/** @dev Changes the address of the the governor.
* @param _governor The address of the new governor.
*/
function changeGovernor(address _governor) external onlyGovernor {

Check notice

Code scanning / Slither

Missing zero address validation Low

Check warning

Code scanning / Slither

Conformance to Solidity naming conventions Warning

//require(msg.sender == governor, "The caller must be the governor.");
governor = _governor;
}
Comment on lines +60 to +63

Check notice

Code scanning / Slither

Missing events access control Low



/** @dev Returns true if the account corresponds to a claimed humanity.
* @param _account The account address.
* @return Whether the account is registered or not.
*/
function isHuman(address _account) public view returns (bool) {

Check warning

Code scanning / Slither

Conformance to Solidity naming conventions Warning

Parameter ProofOfHumanityProxyV2.isHuman(address)._account is not in mixedCase
return proofOfHumanity.isHuman(_account);
}

// ******************** //
// * IERC20 * //
// ******************** //

/** @dev Returns the balance of a particular account of the ProofOfHumanity contract.
* Note that this function takes the expiration date into account.
* @param _account The account address.
* @return The balance of the account.
*/
function balanceOf(address _account) external view returns (uint256) {

Check warning

Code scanning / Slither

Conformance to Solidity naming conventions Warning

return isHuman(_account) ? 1 : 0;
}

/** @dev Returns the count of all humanities that made a registration request at some point.
* Note that with the current implementation of ProofOfHumanity it'd be very costly to count only the humanities that are currently registered.
* @return The total count of humanities.
*/
function totalSupply() external view returns (uint256) {
return proofOfHumanity.getHumanityCount();
}

function transfer(address _recipient, uint256 _amount) external pure returns (bool) { return false; }

function allowance(address _owner, address _spender) external view returns (uint256) {}

function approve(address _spender, uint256 _amount) external pure returns (bool) { return false; }

function transferFrom(address _sender, address _recipient, uint256 _amount) external pure returns (bool) { return false; }
}
Loading
Loading