CodeHawks Escrow Contract - Competition Details

Escrow token balance check is redundant

Summary

This check in the Escrow constructor is redundant:
if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();

Vulnerability Details

The check if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();, along with associated tests, can be removed. Reason being, it checks if the token balance of the Escrow is less than the price. However, the check if (arbiterFee >= price) already ensures that price can never be 0.

price is the amount of tokens that get transferred to the Escrow contract. If arbiter fee is 0 and price is also 0, the check if (arbiterFee >= price) fails. If arbiter fee is more than 0 but price is still 0, the check fails again. Only if the price, i.e., the tokens in Escrow, is greater than the arb fee (which can be 0), will the check pass. Hence, the check Escrow__MustDeployWithTokenBalance() is redundant.

Impact

Gas

Tools Used

Forge, Foundry Toolkit

Recommendation

Remove the check if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();. With this change, an overall reduction of 38201 gas was seen across tests, whereas a saving of 1043 gas was seen on Escrow deployment.