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

Use nested `if` statements instead of logical AND (`&&`)

hasanza

Summary

Compared to nested if statements, using logical AND (&&) results in a relatively larger deployment size, and higher gas overhead on each call.

Vulnerability Details

Inside the onlyBuyerOrSeller modifier, a logical AND (&&) has been used to ensure both conditions (that caller is either buyer or seller) are met for the modified function to be accessed. However, using a logical AND is more expensive than using a nested if statement. This is due to how the compiler optimizes for nested if statements.

/// @dev Throws if called by any account other than buyer or seller.
modifier onlyBuyerOrSeller() {
if (msg.sender != i_buyer && msg.sender != i_seller) {
revert Escrow__OnlyBuyerOrSeller();
}
_;
}

Impact

Gas

Tools Used

Forge, Foundry Tooklit (gas snapshots and gas reports)

Recommendation

The modifier can be refactored to use a nested if statement:

/// @dev Throws if called by any account other than buyer or seller.
modifier onlyBuyerOrSeller() {
if (msg.sender != i_buyer) {
if (msg.sender != i_seller) {
revert Escrow__OnlyBuyerOrSeller();
}
}
_;
}

With this change, the following gas savings were seen:

  • On average, ~1800 gas was saved in each Escrow test in EscrowTest.t.sol, whereas a cumulative 41650 gas saving was seen across all tests.

  • Deployment size was reduced from 3666 to 3657 (a saving of 9 bytes).

  • Deployment cost reduced from 591900 to 590100 (a saving of 1800 gas).

  • initiateDispute (the function using the modifier in question)'s size reduced from 316 bytes to 292 bytes (24 bytes). Whereas calling the function saved 19 gas on each call.

Support

FAQs

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