When deploying a new Escrow via the factory, we can check if the set price is not 0 before interacting with the IERC20 via external calls. This ultimately saves the user some gas.
As it stands, the factory contract performs the token transfer regardless of whether the price is 0 or not. There are currently no checks to ensure that price is always greater than 0, so it can be assumed that a price of 0 is a legal choice (please see my other finding that states a logic flaw resulting in a DoS for the user).
If price is set to 0, the IERC20 interaction will still occur and ultimately waste some gas for the caller. There are 2 interactions during escrow creation that can be tweaked to reduce the overall cost:
EscrowFactory.sol:newEscrow()
Escrow.sol:constructor()
A quick 'forge test --gas-report' shows the following (extracted from the overall results) for the code before the patch:
Additional gas costs occur even if the price is set to 0. These can be avoided.
VS Code
Foundry
Manually reading
This can be addressed by simply wrapping the two identified called with a "not zero" check. In some cases, using != instead of > for the 0 check can also be more efficient. These have been used in this remediation example:
EscrowFactory.sol:newEscrow()
Escrow.sol:constructor()
The following shows the same gas report after the patch, as can be seen, the majority of results are reduced:
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.