CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: gas
Valid

[G‑04] Redundant zero address checks

Arz

All optimizations were benchmarked using the protocol's tests using the following config: solc version 0.8.18, optimizer on, 200 runs, viaIR = true. In most cases forge test --gas-report was used

Each optimization was submitted individually.

Gas Optimizations

Issue Instances Total Gas Saved
[G‑01] Consider using clones * -70% cheaper deployment gas
[G‑02] ReentrancyGuard can be safely removed 1 42725
[G‑03] computeEscrowAddress() can be internal instead of public 1 55863 + 193
[G‑04] Redundant zero address checks 2 237
[G‑05] Input validation should be done in the beginning 2 110649(in the revert case)
[G‑06] Emit after the transfer has been made in case it fails 2 1381(in the revert case)
[G‑07] The bytecode can be removed from the function params 1 27
[G‑08] Nested ifs are cheaper than && 1 19

[G‑04] Redundant zero address checks

When a new escrow is deployed, in the constructor there are 2 zero address checks that are redundant and that only waste gas. The first one is the zero token address check and the second one is the zero address buyer check.

The zero token address is redundant because safeTransferFrom() is used and if the address is zero then this will revert and later it also uses balanceOf which would also revert.

The buyer zero address check is redundant because its impossible for the msg.sender to be address(0) when calling newEscrow().

Gas Savings for newEscrow() obtained via protocol's tests(median was used because the avg isnt accurate here): 237 gas

MED
Before 627057
After 626820

There are 2 instances of this issue:

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L40-L41

File: EscrowFactory.sol
​
39: tokenContract.safeTransferFrom(msg.sender, computedAddress, price);
40: Escrow escrow = new Escrow{salt: salt}(
41: price,
42: tokenContract,
43: msg.sender,
44: seller,
45: arbiter,
46: arbiterFee
47: );
​

As you can see here safeTransferFrom is called so if the tokenContract is address(0) then this would revert. And the buyer cant be address(0) here because msg.sender is used.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.