Note to Judge: Think of this as a combination of a QA, Gas and Analysis report from code4rena's submission standards
The escrow contract is used for protocols looking for private audits on the codeHawks platform. It act as a agreement between the protocol and auditor where protocol will first lock funds in the Escrow conrtact and pay auditor once audits are completed.
There are 3 actors in the project, buyers, sellers and arbiter. The details can be viewed here
confirmedReceipt()
: Primary exit point of contract. Buyer pays seller the agreed price for the audit completed. This function can be called only be called by the buyer
initiateDispute()
: Buyer or Seller can initiate dispute anytime if there are disagreements, but requires a arbiter set by buyer
resolveDispute()
: Secondary exit point of contract. If a dispute is initiated via initiateDispute()
, only arbiter can call this function to resolve dispute for a fee paid by buyer and return/pay funds to buyer/seller
resolveDispute()
: If a dispute is initiated, it cannot be cancelled, and requires arbiter to retrieve funds for buyer. Arbiters can then ultimately decide the amount of funds to send to buyer (protocol) and seller (auditor).
confirmReceipt()
: If no arbiter is set, the outflow of funds ultimately depends on this function. The interesting thing here is that based on the design of the escrow contract, buyer cannot purposely withold funds if no arbiter is set as this function is the only way to transfer funds out and the only recipient is the seller. If not, buyer funds transferred is locked forever in escrow contract.
Consider implementing a blacklist of malicious tokens or only whitelist accepted tokens such as USDC, USDT and trusted project tokens. This also protects against tokens with other caveats such as rebasing and fee-on-transfer tokens that can affect seller's and arbiter's payment.
Currently, the inititateDispute()
function allows dispute to be initiated at anytime by buyer or seller. This allows seller to immediately call the function to force buyer to delegate escrow to arbiter and force the payment of i_arbiterFee
if it exists, even if not desired by buyer.
Consider only allowing the initiation of dispute after a delay. This delay could represent the duration of the audit performed.
Consider supporting multiple auditors (sellers) so that a single escrow contract can delegate to multiple auditors without the need for re-deployment of escrow contract. An example use-case could be a private audit/audit-competition with different, non-affliated auditors.
Consider adding an additional onlyBuyers
function that allows confirming recipt and sending of funds to multiple sellers. An state variable of an array of addresses representing sellers can be assigned in the constructor but cannot be altered any more once assigned.
i_price
Lack of emergency withdraw option would mean in the event that buyer mistakenly sends more tokens then expected, best case it will require arbiter to retrieve the funds for them. Worse case, if no arbiter is set, funds are locked forever unless buyer calls confirmReceipt()
to send funds to seller.
Use a stricter check in the constructor
i_seller
and i_tokenContract
in confirmReceipt()
Caching i_seller
and i_tokenContract
saves SLOADs when loading for event emission and safeTransfer()
.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.