Leisurepay

The `Dispute` contract inherits from OpenZeppelin's `AccessControlEnumerable`. Within `Dispute`, the role `SERVER_ROLE` is set up to be used as a privileged role through the inherited contract. Note that the `SERVER_ROLE` is not defined as suggested in OpenZeppelin's file.

The ERC20 token `lpy` is specified at deployment of the `Dispute` contract. This is understood to be an in-house token. However, the token contract is out of scope for this audit, and as such, its functional correctness is assumed. In reality, if the `transfer()` function does not work as expected, then it may cause issues within the contract `Dispute`. Moreover, note that unless `lpy` tokens are transferred independently to the `Dispute` contract via an external method, this contract will not have a balance available to support this transfer, as no part of the contract's logic acquires tokens on behalf of the contract. A similar issue arises with the address `_nftAddr` used in `createDisputeByServer()`. Again, it is understood from context that the corresponding `_nftAddr` is meant to be in-house. Since the contract is out of scope for the audit, its functional correctness is assumed.

The `Dispute` contract inherits from OpenZeppelin's `AccessControlEnumerable`, meaning multiple addresses may take on the `SERVER_ROLE` at once; the `DEFAULT_ADMIN_ROLE` (the deployer of the `Dispute` contract) can assign as many `SERVER_ROLE` accounts as they choose.

Arbiters vote on a dispute through function `castVote()`. They do so by input a bool `_agree`. The comments above this function specify that `true==YES` and `false==NO`. However, in function `_getSignerAddress()`, an internal function called when the `SERVER_ROLE` uses signatures to cast votes on behalf of arbiters, the functionality suggests that a bool of `true` for `_agree` is a vote in favor of `sideA` being cast as the `winner` of the dispute.

Function `claim()` has a TODO comment stating `TODO: USE TRANSFER FROM HERE.` Please provide clarification on the following: - Is this comment specifying that the intention is to replace `transfer()` with `transferFrom()`? If so, please clarify who the `lpy` tokens will be transferred from. If the `lpy` tokens are to be transferred from the losing side address, is appears nothing enforces the losing address to approve the transfer of tokens to the caller of `claim()`.

The following comment is not accurate: ```solidity /// @notice Adds a new dispute to memory ``` The function `createDisputeByServer()` is adding a new dispute to the mapping in storage.

Functions that affect the state of storage variables should emit events.

The following input values are missing relevant validation checks: - In function `createDisputeByServer()`, input `usdValue` is missing a check that it is nonzero. - In function `finalizeDispute()`, input `ratio` is missing a check that it is nonzero. - In functions `updateSideA()` and `updateSideB()`, there is a missing check that `dispute.state == State.Open`.

Addresses should be checked before assignment or external calls to make sure they are not `address(0)`.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. *This finding is considered minor because the reentrancy only causes out-of-order events.* ### External call(s) ```solidity=535 require( lpy.transfer(msg.sender, _dispute.tokenValue), "CLAIM:: transfer failed" ); ``` ### Events emitted after the call(s) ```solidity=540 emit DisputeFundClaimed(_dispute.disputeIndex, _dispute.tokenValue, msg.sender); ```