SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: high

Replace require statements with already defined custom errors to reduce gas costs

Author Revealed upon completion

Root + Impact

Description

  • Currently, the TreasureHunt.sol contract is built with a lot of require statements with string messages alongside some usage of reverts with custom errors. In the meantime, several predefined custom errors remain unused.

  • Replace the require statements with the already defined custom errors to save on gas and achieve better code maintainability.

// 1. withdraw() function - Line 212
@> require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
​
// 2. withdraw() function - Line 215
@> require(balance > 0, "NO_FUNDS_TO_WITHDRAW");
​
// 3. fund() function - Line 224
@> require(msg.sender == owner, "ONLY_OWNER_CAN_FUND");
​
// 4. pause() function - Line 232
@> require(msg.sender == owner, "ONLY_OWNER_CAN_PAUSE");
​
// 5. unpause() function - Line 240
@> require(msg.sender == owner, "ONLY_OWNER_CAN_UNPAUSE");
​
// 6. updateVerifier() function - Line 248
@> require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
​
// 7. updateVerifier() function - Line 249
@> require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
​
// 8. emergencyWithdraw() function - Line 265
@> require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
​
// 9. emergencyWithdraw() function - Line 266
@> require(msg.sender == owner, "ONLY_OWNER_CAN_EMERGENCY_WITHDRAW");
​
// 10. emergencyWithdraw() function - Lines 273-276
@> require(
@> amount > 0 && amount <= address(this).balance,
@> "INVALID_AMOUNT"
@> );

Risk

Likelihood:

  • Always: Each time the contract is deployed and when a require statement reverts with a custom string message, significantly more gas will be spent in comparison to reverts with custom errors.


Impact:

  • Low: This is a gas optimization and code maintainability upgrade. It does not affect the core functionality of the contract.


Proof of Concept


  • Reduce deployment and execution gas cost by replacing the require statements with custom errors.

  • Calling the withdraw function with custom errors costs less gas compared to calling the same function with require statements.

function withdraw() external {
if (claimsCount < MAX_TREASURES) revert HuntNotOver();
​
uint256 balance = address(this).balance;
if (balance == 0) revert NoFundsToWithdraw();
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
​
emit Withdrawn(balance, address(this).balance);
}

Recommended Mitigation

  • Replace each require statement with the corresponding custom error

--- a/contracts/src/TreasureHunt.sol
+++ b/contracts/src/TreasureHunt.sol
​
@@ -209,10 +209,10 @@
/// @notice Allow the owner to withdraw unclaimed funds after the hunt is over.
function withdraw() external {
- require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
+ if (claimsCount < MAX_TREASURES) revert HuntNotOver();
uint256 balance = address(this).balance;
- require(balance > 0, "NO_FUNDS_TO_WITHDRAW");
+ if (balance == 0) revert NoFundsToWithdraw();
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
@@ -221,7 +221,7 @@
/// @notice Allow the owner to add more funds if needed.
function fund() external payable {
- require(msg.sender == owner, "ONLY_OWNER_CAN_FUND");
+ if (msg.sender != owner) revert OnlyOwnerCanFund();
require(msg.value > 0, "NO_ETH_SENT");
emit Funded(msg.value, address(this).balance);
@@ -229,7 +229,7 @@
/// @notice Pause the contract.
function pause() external {
- require(msg.sender == owner, "ONLY_OWNER_CAN_PAUSE");
+ if (msg.sender != owner) revert OnlyOwnerCanPause();
paused = true;
emit Paused(msg.sender);
@@ -237,7 +237,7 @@
/// @notice Unpause the contract.
function unpause() external {
- require(msg.sender == owner, "ONLY_OWNER_CAN_UNPAUSE");
+ if (msg.sender != owner) revert OnlyOwnerCanUnpause();
paused = false;
emit Unpaused(msg.sender);
@@ -245,8 +245,8 @@
/// @notice In case of a bug, allow the owner to update the verifier address.
function updateVerifier(IVerifier newVerifier) external {
- require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
- require(msg.sender == owner, "ONLY_OWNER_CAN_UPDATE_VERIFIER");
+ if (!paused) revert TheContractMustBePaused();
+ if (msg.sender != owner) revert OnlyOwnerCanUpdateVerifier();
if (
address(newVerifier) == address(0) ||
address(newVerifier) == address(verifier)
@@ -262,8 +262,8 @@
address payable recipient,
uint256 amount
) external {
- require(paused, "THE_CONTRACT_MUST_BE_PAUSED");
- require(msg.sender == owner, "ONLY_OWNER_CAN_EMERGENCY_WITHDRAW");
+ if (!paused) revert TheContractMustBePaused();
+ if (msg.sender != owner) revert OnlyOwnerCanEmergencyWithdraw();
require(
recipient != address(0) &&
recipient != address(this) &&
recipient != owner,
"INVALID_RECIPIENT"
);
- require(
- amount > 0 && amount <= address(this).balance,
- "INVALID_AMOUNT"
- );
+ if (amount == 0 || amount > address(this).balance) revert InvalidAmount();
(bool sent, ) = recipient.call{value: amount}("");
require(sent, "ETH_TRANSFER_FAILED");
​

Support

FAQs

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

Give us feedback!