Juicebox V2 contest - Metatron's results

The decentralized fundraising and treasury protocol.

General Information

Platform: Code4rena

Start Date: 01/07/2022

Pot Size: $75,000 USDC

Total HM: 17

Participants: 105

Period: 7 days

Judge: Jack the Pug

Total Solo HM: 5

Id: 143

League: ETH

Juicebox

Findings Distribution

Researcher Performance

Rank: 89/105

Findings: 1

Award: $38.24

🌟 Selected for report: 0

🚀 Solo Findings: 0

[G-01] ++i costs less gas compared to i++

++i costs about 5 gas less per iteration compared to i++ for unsigned integer. This statement is true even with the optimizer enabled. Summarized my results where i\ _i\ j\ _j is used in a loop, is unsigned integer, and you safely can be changed to ++i without changing any behavior.

I've found 15 locations in 6 files:

contracts/JBController.sol:
   912      //Transfer between all splits.
   913:     for (uint256 _i = 0; _i < _splits.length; _i++) {
   914        // Get a reference to the split being iterated on.

  1013      // Set distribution limits if there are any.
  1014:     for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {
  1015        JBFundAccessConstraints memory _constraints = _fundAccessConstraints[_i];

contracts/JBDirectory.sol:
  138      // Return the first terminal which accepts the specified token.
  139:     for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {
  140        IJBPaymentTerminal _terminal = _terminalsOf[_projectId][_i];

  166    {
  167:     for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)
  168        if (_terminalsOf[_projectId][_i] == _terminal) return true;

  274      if (_terminals.length > 1)
  275:       for (uint256 _i; _i < _terminals.length; _i++)
  276:         for (uint256 _j = _i + 1; _j < _terminals.length; _j++)
  277            if (_terminals[_i] == _terminals[_j]) revert DUPLICATE_TERMINALS();

contracts/JBFundingCycleStore.sol:
  723  
  724:     for (uint256 i = 0; i < _discountMultiple; i++) {
  725        // The number of times to apply the discount rate.

contracts/JBOperatorStore.sol:
   84    ) external view override returns (bool) {
   85:     for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {
   86        uint256 _permissionIndex = _permissionIndexes[_i];

  134    function setOperators(JBOperatorData[] calldata _operatorData) external override {
  135:     for (uint256 _i = 0; _i < _operatorData.length; _i++) {
  136        // Pack the indexes into a uint256.

  164    function _packedPermissions(uint256[] calldata _indexes) private pure returns (uint256 packed) {
  165:     for (uint256 _i = 0; _i < _indexes.length; _i++) {
  166        uint256 _index = _indexes[_i];

contracts/JBSingleTokenPaymentTerminalStore.sol:
  861      // Add the current ETH overflow for each terminal.
  862:     for (uint256 _i = 0; _i < _terminals.length; _i++)
  863        _ethOverflow = _ethOverflow + _terminals[_i].currentEthOverflowOf(_projectId);

contracts/JBSplitsStore.sol:
  203      // Check to see if all locked splits are included.
  204:     for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
  205        // If not locked, continue.

  210  
  211:       for (uint256 _j = 0; _j < _splits.length; _j++) {
  212          // Check for sameness.

  228  
  229:     for (uint256 _i = 0; _i < _splits.length; _i++) {
  230        // The percent should be greater than 0.

  303      // Loop through each split and unpack the values into structs.
  304:     for (uint256 _i = 0; _i < _splitCount; _i++) {
  305        // Get a reference to the fist packed data.

[G-02] An arrays length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset). Caching the array length in the stack saves around 3 gas per iteration.

I've found 14 locations in 6 files

contracts/JBController.sol:
   912      //Transfer between all splits.
   913:     for (uint256 _i = 0; _i < _splits.length; _i++) {
   914        // Get a reference to the split being iterated on.

  1013      // Set distribution limits if there are any.
  1014:     for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {
  1015        JBFundAccessConstraints memory _constraints = _fundAccessConstraints[_i];

contracts/JBDirectory.sol:
  138      // Return the first terminal which accepts the specified token.
  139:     for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {
  140        IJBPaymentTerminal _terminal = _terminalsOf[_projectId][_i];

  166    {
  167:     for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)
  168        if (_terminalsOf[_projectId][_i] == _terminal) return true;

  274      if (_terminals.length > 1)
  275:       for (uint256 _i; _i < _terminals.length; _i++)
  276:         for (uint256 _j = _i + 1; _j < _terminals.length; _j++)
  277            if (_terminals[_i] == _terminals[_j]) revert DUPLICATE_TERMINALS();

contracts/JBOperatorStore.sol:
   84    ) external view override returns (bool) {
   85:     for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {
   86        uint256 _permissionIndex = _permissionIndexes[_i];

  134    function setOperators(JBOperatorData[] calldata _operatorData) external override {
  135:     for (uint256 _i = 0; _i < _operatorData.length; _i++) {
  136        // Pack the indexes into a uint256.

  164    function _packedPermissions(uint256[] calldata _indexes) private pure returns (uint256 packed) {
  165:     for (uint256 _i = 0; _i < _indexes.length; _i++) {
  166        uint256 _index = _indexes[_i];

contracts/JBSingleTokenPaymentTerminalStore.sol:
  861      // Add the current ETH overflow for each terminal.
  862:     for (uint256 _i = 0; _i < _terminals.length; _i++)
  863        _ethOverflow = _ethOverflow + _terminals[_i].currentEthOverflowOf(_projectId);

contracts/JBSplitsStore.sol:
  203      // Check to see if all locked splits are included.
  204:     for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
  205        // If not locked, continue.

  210  
  211:       for (uint256 _j = 0; _j < _splits.length; _j++) {
  212          // Check for sameness.

  228  
  229:     for (uint256 _i = 0; _i < _splits.length; _i++) {
  230        // The percent should be greater than 0.

contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol:
  1007      // Transfer between all splits.
  1008:     for (uint256 _i = 0; _i < _splits.length; ) {
  1009        // Get a reference to the split being iterated on.

[G-03] Using default values is cheaper than assignment

If a variable is not set/initialized, it is assumed to have the default value, 0 for uint Explicitly initializing it with its default value is an anti-pattern and wastes gas. For example: uint256 i = 0; should be replaced with uint256 i;

I've found 15 locations in 6 files

contracts/JBController.sol:
   912      //Transfer between all splits.
   913:     for (uint256 _i = 0; _i < _splits.length; _i++) {
   914        // Get a reference to the split being iterated on.

contracts/JBFundingCycleStore.sol:
  723  
  724:     for (uint256 i = 0; i < _discountMultiple; i++) {
  725        // The number of times to apply the discount rate.

contracts/JBOperatorStore.sol:
   84    ) external view override returns (bool) {
   85:     for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {
   86        uint256 _permissionIndex = _permissionIndexes[_i];

  134    function setOperators(JBOperatorData[] calldata _operatorData) external override {
  135:     for (uint256 _i = 0; _i < _operatorData.length; _i++) {
  136        // Pack the indexes into a uint256.

  164    function _packedPermissions(uint256[] calldata _indexes) private pure returns (uint256 packed) {
  165:     for (uint256 _i = 0; _i < _indexes.length; _i++) {
  166        uint256 _index = _indexes[_i];

contracts/JBSingleTokenPaymentTerminalStore.sol:
  861      // Add the current ETH overflow for each terminal.
  862:     for (uint256 _i = 0; _i < _terminals.length; _i++)
  863        _ethOverflow = _ethOverflow + _terminals[_i].currentEthOverflowOf(_projectId);

contracts/JBSplitsStore.sol:
  164      // Set each grouped splits.
  165:     for (uint256 _i = 0; _i < _groupedSplitsLength; ) {
  166        // Get a reference to the grouped split being iterated on.

  203      // Check to see if all locked splits are included.
  204:     for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
  205        // If not locked, continue.

  210  
  211:       for (uint256 _j = 0; _j < _splits.length; _j++) {
  212          // Check for sameness.

  226      // Add up all the percents to make sure they cumulative are under 100%.
  227:     uint256 _percentTotal = 0;
  228  
  229:     for (uint256 _i = 0; _i < _splits.length; _i++) {
  230        // The percent should be greater than 0.

  303      // Loop through each split and unpack the values into structs.
  304:     for (uint256 _i = 0; _i < _splitCount; _i++) {
  305        // Get a reference to the fist packed data.

contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol:
   593      // Process each fee.
   594:     for (uint256 _i = 0; _i < _heldFeeLength; ) {
   595        // Get the fee amount.

  1007      // Transfer between all splits.
  1008:     for (uint256 _i = 0; _i < _splits.length; ) {
  1009        // Get a reference to the split being iterated on.

  1395      // Process each fee.
  1396:     for (uint256 _i = 0; _i < _heldFeesLength; ) {
  1397        if (leftoverAmount == 0) _heldFeesOf[_projectId].push(_heldFees[_i]);

[G-04] != 0 is cheaper than > 0

!= 0 costs less gas compared to > 0 for unsigned integers even when optimizer enabled All of the following findings are uint (E&OE) so >0 and != have exectly the same effect. ** saves 6 gas ** each

I've found 33 locations in 7 files:

contracts/JBController.sol:
   437      // Add the provided terminals to the list of terminals.
   438:     if (_terminals.length > 0) directory.setTerminalsOf(projectId, _terminals);
   439  

   480      // If there is a previous configuration, reconfigureFundingCyclesOf should be called instead
   481:     if (fundingCycleStore.latestConfigurationOf(_projectId) > 0)  //@audit latestConfigurationOf() returns uint256
   482        revert FUNDING_CYCLE_ALREADY_LAUNCHED();

   497      // Add the provided terminals to the list of terminals.
   498:     if (_terminals.length > 0) directory.setTerminalsOf(_projectId, _terminals);
   499  

   874      // Mint any leftover tokens to the project owner.
   875:     if (_leftoverTokenCount > 0) tokenStore.mintFor(_owner, _projectId, _leftoverTokenCount, false);
   876  

   924        // Mints tokens for the split if needed.
   925:       if (_tokenCount > 0) {
   926          tokenStore.mintFor(

  1031        // Set the distribution limit if there is one.
  1032:       if (_constraints.distributionLimit > 0)  //@audit all relevant members of JBFundAccessConstraints are uints
  1033          _packedDistributionLimitDataOf[_projectId][_fundingCycle.configuration][

  1039        // Set the overflow allowance if there is one.
  1040:       if (_constraints.overflowAllowance > 0)
  1041          _packedOverflowAllowanceDataOf[_projectId][_fundingCycle.configuration][

contracts/JBFundingCycleStore.sol:
  148      // If it exists, return its funding cycle if it is approved.
  149:     if (_standbyFundingCycleConfiguration > 0) {
  150        fundingCycle = _getStructFor(_projectId, _standbyFundingCycleConfiguration);

  209      // If an eligible funding cycle exists...
  210:     if (_fundingCycleConfiguration > 0) {
  211        // Resolve the funding cycle for the eligible configuration.

  346        _data.ballot != IJBFundingCycleBallot(address(0)) ||
  347:       _data.duration > 0 ||
  348:       _data.discountRate > 0
  349      ) {

  363      // Set the metadata if needed.
  364:     if (_metadata > 0) _metadataOf[_projectId][_configuration] = _metadata;
  365  

  468        // This is to allow a weight of 0 (default) to represent inheriting the discounted weight of the previous funding cycle.
  469:       _weight = _weight > 0
  470          ? (_weight == 1 ? 0 : _weight)

  559      if (
  560:       _baseFundingCycle.duration > 0 &&
  561        block.timestamp < _fundingCycle.start - _baseFundingCycle.duration

  588      if (
  589:       _fundingCycle.duration > 0 && block.timestamp >= _fundingCycle.start + _fundingCycle.duration
  590      ) return 0;

  600      if (
  601:       _baseFundingCycle.duration > 0 &&
  602        block.timestamp >= _baseFundingCycle.start + _baseFundingCycle.duration

contracts/JBProjects.sol:
  142      // Set the metadata if one was provided.
  143:     if (bytes(_metadata.content).length > 0)
  144        metadataContentOf[projectId][_metadata.domain] = _metadata.content;

contracts/JBSingleTokenPaymentTerminalStore.sol:
  474  
  475:         if (_currentOverflow > 0)
  476            // Calculate reclaim amount using the current overflow amount.

  517      // Remove the reclaimed funds from the project's balance.
  518:     if (reclaimAmount > 0)
  519        balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] =

contracts/JBSplitsStore.sol:
  258        // If there's data to store in the second packed split part, pack and store.
  259:       if (_splits[_i].lockedUntil > 0 || _splits[_i].allocator != IJBSplitAllocator(address(0))) {
  260          // Locked until should be within a uint48

  271          // Otherwise if there's a value stored in the indexed position, delete it.
  272:       } else if (_packedSplitParts2Of[_projectId][_domain][_group][_i] > 0)
  273          delete _packedSplitParts2Of[_projectId][_domain][_group][_i];

  325        // If there's anything in it, unpack.
  326:       if (_packedSplitPart2 > 0) {
  327          // lockedUntil in bits 0-47.

contracts/JBTokenStore.sol:
  355      // Subtract the tokens from the unclaimed balance and total supply.
  356:     if (_unclaimedTokensToBurn > 0) {
  357        // Reduce the holders balance and the total supply.

  366      // Burn the claimed tokens.
  367:     if (_claimedTokensToBurn > 0) _token.burn(_projectId, _holder, _claimedTokensToBurn);
  368  

contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol:
   345      if (token != JBTokens.ETH) {
   346:       if (msg.value > 0) revert NO_MSG_VALUE_ALLOWED();
   347  

   515      // Transfer the balance if needed.
   516:     if (balance > 0) {
   517        // Trigger any inherited pre-transfer logic.

   551        // Amount must be greater than 0.
   552:       if (msg.value > 0) revert NO_MSG_VALUE_ALLOWED();
   553  

   744        // Burn the project tokens.
   745:       if (_tokenCount > 0)
   746          IJBController(directory.controllerOf(_projectId)).burnTokensOf(

   771      // Send the reclaimed funds to the beneficiary.
   772:     if (reclaimAmount > 0) _transferFrom(address(this), _beneficiary, reclaimAmount);
   773  

   883        // Transfer any remaining balance to the project owner.
   884:       if (netLeftoverDistributionAmount > 0)
   885          _transferFrom(address(this), _projectOwner, netLeftoverDistributionAmount);

   963        // Transfer any remaining balance to the beneficiary.
   964:       if (netDistributedAmount > 0)
   965          _transferFrom(address(this), _beneficiary, netDistributedAmount);

  1021  
  1022:       if (_payoutAmount > 0) {
  1023          // Transfer tokens to the split.

  1296        // Mint the tokens if needed.
  1297:       if (_tokenCount > 0)
  1298          // Set token count to be the number of tokens minted for the beneficiary instead of the total amount.

[G-05] Upgrade pragma to 0.8.15 to save gas

Across the whole solution, the declared pragma is 0.8.6 Upgrading to 0.8.15 has many benefits, cheaper on gas is one of them. The following information is regarding 0.8.15 over 0.8.14. Assume that over 0.8.6 the save is higher!

According to the release note of 0.8.15: https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/ The benchmark shows saving of 2.5-10% Bytecode size, Saving 2-8% Deployment gas, And saving up to 6.2% Runtime gas.
AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter