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

Gas Optimization: Inline nonReentrant modifier invoked only once

Author Revealed upon completion

Root + Impact

Description

  • Modifiers are normally utilized to prevent code duplication by extracting reusable logic that applies to multiple functions across a contract.

  • The nonReentrant modifier in the TreasureHunt contract is defined but only utilized once within the claim function. This implementation creates unnecessary jumps during execution and increases the overall bytecode size without providing any reusability benefits.

// Root cause in the codebase with @> marks to highlight the relevant section
@> modifier nonReentrant() {
@> require(!locked, "REENTRANCY_GUARD");
@> locked = true;
@> _;
@> locked = false;
@> }

Risk

Likelihood:

  • The modifier overhead is consistently applied every time the contract is compiled and deployed.

  • The extra jump instruction is executed every time the claim function is called.

Impact:

  • Increased deployment gas cost due to the larger contract bytecode size.

  • Marginally higher execution gas cost when calling the claim function.

Proof of Concept

Explanation: The nonReentrant modifier is exclusively attached to the claim function. Reviewing the entire contract confirms it is not reused in any other function. Therefore, the abstraction overhead provides zero reusability value.

// The modifier is only attached to this single function
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
if (paused) revert ContractPaused();
// ... function logic
}

Recommended Mitigation

Explanation: Remove the nonReentrant modifier declaration entirely and inline its logic (require check and locked boolean toggling) directly inside the claim function body. This approach maintains the exact same reentrancy protection while eliminating the modifier's jump overhead, which reduces both deployment and execution gas costs.

- modifier nonReentrant() {
- require(!locked, "REENTRANCY_GUARD");
- locked = true;
- _;
- locked = false;
- }
​
- function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
+ function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external {
+ require(!locked, "REENTRANCY_GUARD");
+ locked = true;
+
if (paused) revert ContractPaused();
if (address(this).balance < REWARD) revert NotEnoughFunds();
// ... [keep existing internal logic] ...
emit Claimed(treasureHash, msg.sender);
+ locked = false;
}

Support

FAQs

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

Give us feedback!