Juicebox V2 contest - IllIllI'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: 4/105

Findings: 8

Award: $5,370.84

🌟 Selected for report: 2

πŸš€ Solo Findings: 0

Awards

14.8726 USDC - $14.87

Labels

bug
duplicate
3 (High Risk)
old-submission-method
valid

External Links

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBPrices.sol#L69 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBChainlinkV3PriceFeed.sol#L42-L44

Vulnerability details

Impact

If the oracle price feeds are insufficiently validated, there will be pricing errors leading to the miss-pricing of assets

Proof of Concept

The JBSingleTokenPaymentTerminalStore and abstract JBPayoutRedemptionPaymentTerminal both rely on their respective internal prices objects for determining the exchange rate between the tokens that customers provide to the terminals, and the project tokens minted for them. The prices objects within these contracts internally use IJBPriceFeeds to fetch their prices:

File: contracts/JBPrices.sol   #1

69       if (_feed != IJBPriceFeed(address(0))) return _feed.currentPrice(_decimals);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBPrices.sol#L69

IJBPriceFeed itself can be anything an impelemnter wishes, but the default provided by the project uses the following Chainlink call to fetch prices:

File: contracts/JBChainlinkV3PriceFeed.sol   #2

42     function currentPrice(uint256 _decimals) external view override returns (uint256) {
43       // Get the latest round information. Only need the price is needed.
44       (, int256 _price, , , ) = feed.latestRoundData();

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBChainlinkV3PriceFeed.sol#L42-L44

The code above does not verify that answeredInRound >= roundID, or that the timestamp is valid either, because the return values are ignored.

Tools Used

Code inspection

Fetch all return values from the call to latestRoundData() and require() that answeredInRound is greater than or equal to roundID, and that the updatedAt timestamp is not equal to zero

#0 - drgorillamd

2022-07-12T18:50:54Z

Duplicate of #138

Findings Information

🌟 Selected for report: zzzitron

Also found by: IllIllI

Labels

bug
duplicate
2 (Med Risk)
sponsor disputed
old-submission-method
valid

Awards

1929.6275 USDC - $1,929.63

External Links

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L504-L526

Vulnerability details

Impact

Most of the configuration parameters are packed into a single uint256 in order for them to be efficiently fetched later. It's possible to provide values that will corrupt the packing, leading to contract accounting to become broken

Proof of Concept

This function handles the packing of the parameters:

File: contracts/JBFundingCycleStore.sol   #1

493     /**
494       @notice 
495       Efficiently stores a funding cycle's provided intrinsic properties.
496   
497       @param _configuration The configuration of the funding cycle to pack and store.
498       @param _projectId The ID of the project to which the funding cycle belongs.
499       @param _number The number of the funding cycle.
500       @param _weight The weight of the funding cycle.
501       @param _basedOn The configuration of the base funding cycle.
502       @param _start The start time of this funding cycle.
503     */
504     function _packAndStoreIntrinsicPropertiesOf(
505       uint256 _configuration,
506       uint256 _projectId,
507       uint256 _number,
508       uint256 _weight,
509       uint256 _basedOn,
510       uint256 _start
511     ) private {
512       // weight in bits 0-87.
513       uint256 packed = _weight;
514   
515       // basedOn in bits 88-143.
516       packed |= _basedOn << 88;
517   
518       // start in bits 144-199.
519       packed |= _start << 144;
520   
521       // number in bits 200-255.
522       packed |= _number << 200;
523   
524       // Store the packed value.
525       _packedIntrinsicPropertiesOf[_projectId][_configuration] = packed;
526     }

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L493-L526

_basedOn comes from a lookup of an existing cycle and is therefore trusted (i.e. not validated). _start trusted for a similar reason, as is _number. _weight is user-provided but is correctly validated.

If _basedOn is equal to type(uint88).max, and the project's duration is non-zero, the calculation for _start will return a value larger than the wanted size:

File: contracts/JBFundingCycleStore.sol   #2

685       // Add increments of duration as necessary to satisfy the threshold.
686       while (_mustStartAtOrAfter > start) start = start + _baseFundingCycle.duration;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L685-L686

Similar issues will of course arise if _basedOn is smaller, but the cycle duration is larger.

This overflow will alter the packed value for _number, filling in the 'or' of the two, so it'll be incorrect. Because the overflow of _start alters earlier bits too, the 'start' stored may actually be a timestamp in the past. If the timestamp is in the past, and there are weights and durations also set for the project, amounts calculated based on these values will be wrong, and users will not get the correct tokens that they're owed. One example, of a few, is here where the calculated weight will be wrong:

File: contracts/JBFundingCycleStore.sol   #3

698     function _deriveWeightFrom(JBFundingCycle memory _baseFundingCycle, uint256 _start)
699       private
700       pure
701       returns (uint256 weight)
702     {
703       // A subsequent cycle to one with a duration of 0 should have the next possible weight.
704       if (_baseFundingCycle.duration == 0)
705         return
706           PRBMath.mulDiv(
707             _baseFundingCycle.weight,
708             JBConstants.MAX_DISCOUNT_RATE - _baseFundingCycle.discountRate,
709             JBConstants.MAX_DISCOUNT_RATE
710           );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L698-L710

These values don't have to be provided by the owner - only the JBOperations.RECONFIGURE role is necessary to be able to trigger these sort of issues.

Tools Used

Code inspection

Add require()s to ensure that the numbers being packed won't overflow their bits

#0 - drgorillamd

2022-07-12T20:07:29Z

funding cycle configurations id are the timestamps when the reconfigure was submitted, a basedOn == type(uint88).max is therefore a reconfiguration submitted at a block.timestamp of 309,485,009,821,345,068,724,781,055

#1 - jack-the-pug

2022-08-07T07:44:07Z

Duplicate of #220

Awards

3.4075 USDC - $3.41

Labels

bug
duplicate
2 (Med Risk)
old-submission-method
valid

External Links

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L73-L89

Vulnerability details

Impact

Not all IERC20 implementations revert() when there's a failure in transfer()/transferFrom(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment.

A user may attempt to pay the terminal using one of these tokens, and if the terminal supports the token, the user can have a zero balance in the token, and the terminal will still credit him/her with the amount stated, leading to tokens being minted for the user when they shouldn't have been.

Proof of Concept

All terminal-related ERC-20 transfer are accomplished through this function, which does not check the return codes of either transfer() or transferFrom():

File: contracts/JBERC20PaymentTerminal.sol   #1

73     /** 
74       @notice
75       Transfers tokens.
76   
77       @param _from The address from which the transfer should originate.
78       @param _to The address to which the transfer should go.
79       @param _amount The amount of the transfer, as a fixed point number with the same number of decimals as this terminal.
80     */
81     function _transferFrom(
82       address _from,
83       address payable _to,
84       uint256 _amount
85     ) internal override {
86       _from == address(this)
87         ? IERC20(token).transfer(_to, _amount)
88         : IERC20(token).transferFrom(_from, _to, _amount);
89     }

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L73-L89

Tools Used

Code inspection

Add require()s ensuring that the transfer calls return success

#0 - drgorillamd

2022-07-12T18:52:56Z

Duplicate of #281

Findings Information

🌟 Selected for report: dirk_y

Also found by: IllIllI, ylv

Labels

bug
duplicate
2 (Med Risk)
old-submission-method
valid

Awards

1157.7765 USDC - $1,157.78

External Links

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L194-L224

Vulnerability details

Impact

If there are enough entries in the splits array, the checks done to ensure existing locks are respected will cause attempts to change the split to revert, preventing the existing split assignment from changing. If the project has a lock with a long duration, the project will have no choice but to spin up another Juicebox project and spin down the old one if they wish to change the splits assignments, which are used to distribute reserved tokens.

Note that this is different from running out of gas while distributing the splits (filed separately) - it's about being unable to change the splits configuration itself.

Proof of Concept

JBSplitsStore._set() uses an O(n^2) algorithm to allow locked splits to extend their lock times:

File: contracts/JBSplitsStore.sol   #1

194     function _set(
195       uint256 _projectId,
196       uint256 _domain,
197       uint256 _group,
198       JBSplit[] memory _splits
199     ) internal {
200       // Get a reference to the project's current splits.
201       JBSplit[] memory _currentSplits = _getStructsFor(_projectId, _domain, _group);
202   
203       // Check to see if all locked splits are included.
204       for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
205         // If not locked, continue.
206         if (block.timestamp >= _currentSplits[_i].lockedUntil) continue;
207   
208         // Keep a reference to whether or not the locked split being iterated on is included.
209         bool _includesLocked = false;
210   
211         for (uint256 _j = 0; _j < _splits.length; _j++) {
212           // Check for sameness.
213           if (
214             _splits[_j].percent == _currentSplits[_i].percent &&
215             _splits[_j].beneficiary == _currentSplits[_i].beneficiary &&
216             _splits[_j].allocator == _currentSplits[_i].allocator &&
217             _splits[_j].projectId == _currentSplits[_i].projectId &&
218             // Allow lock extention.
219             _splits[_j].lockedUntil >= _currentSplits[_i].lockedUntil
220           ) _includesLocked = true;
221         }
222   
223         if (!_includesLocked) revert PREVIOUS_LOCKED_SPLITS_NOT_INCLUDED();
224       }

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L194-L224

If there currently are no splits and enough splits are then added such that adding one more would have caused the function to run out of gas, and there is at least one split that has a value for lockedUntil that is in the future, during the next changing of splits, the algorithm will do a second pass through the array due to the condition on line 206 not being satisfied, and the function will run out of gas. The more locks there are, the fewer splits are required for the function to run out of gas during the next call.

JBSplitStore._set() is called by JBSplitsStore.set():

File: contracts/JBSplitsStore.sol   #2

147     function set(
148       uint256 _projectId,
149       uint256 _domain,
150       JBGroupedSplits[] calldata _groupedSplits
151     )
152       external
153       override
154       requirePermissionAllowingOverride(
155         projects.ownerOf(_projectId),
156         _projectId,
157         JBOperations.SET_SPLITS,
158         address(directory.controllerOf(_projectId)) == msg.sender
159       )
160     {
161       // Push array length in stack
162       uint256 _groupedSplitsLength = _groupedSplits.length;
163   
164       // Set each grouped splits.
165       for (uint256 _i = 0; _i < _groupedSplitsLength; ) {
166         // Get a reference to the grouped split being iterated on.
167         JBGroupedSplits memory _groupedSplit = _groupedSplits[_i];
168   
169         // Set the splits for the group.
170         _set(_projectId, _domain, _groupedSplit.group, _groupedSplit.splits);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L147-L170

which is called by the Controller:

File: contracts/JBController.sol   #3

1011       splitsStore.set(_projectId, _fundingCycle.configuration, _groupedSplits);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L1011

on the immutable split store, which means once the issue is hit, it's can't be recovered from until the last lock expires:

File: contracts/JBController.sol   #4

129     IJBSplitsStore public immutable override splitsStore;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L129

Migration cannot be used to avoid having to spin up another JuiceBox project, because if the function runs out of gas while trying to do operations on objects in memory, it surely will run out of gas when iterating over the same operations with the external calls made for minting each split, which will add a minimum overhead of 100 gas per call:

File: contracts/JBController.sol   #5

815       // All reserved tokens must be minted before migrating.
816       if (uint256(_processedTokenTrackerOf[_projectId]) != tokenStore.totalSupplyOf(_projectId))
817         _distributeReservedTokensOf(_projectId, '');

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L815-L817

Tools Used

Code inspection

The easiest solution is to not allow splits to change unless all locks have expired. An exact but expensive method (in terms of gas) would be to have a function that decides the maximum number of splits based on the number of locked splits, and reject changes that violate that maximum. Note that unless the maximum number of locks is compiled to be a very small number, the function will have to do the calculation from scratch every time, or memoize answers, which will waste a lot of gas. A less expensive, but less flexible solution is to have a very low maximum number of splits.

#0 - drgorillamd

2022-07-12T18:56:47Z

Duplicate of #170

Findings Information

🌟 Selected for report: IllIllI

Also found by: Meera, cccz, hake, rbserver, robee

Labels

bug
documentation
2 (Med Risk)
sponsor acknowledged
old-submission-method
valid

Awards

422.0095 USDC - $422.01

External Links

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L817-L856

Vulnerability details

Some ERC20 tokens, such as USDT, allow for charging a fee any time transfer() or transferFrom() is called. If a contract does not allow for amounts to change after transfers, subsequent transfer operations based on the original amount will revert() due to the contract having an insufficient balance.

Impact

If there is only one user that has use a payment terminal with a fee-on-transfer token to pay a project for its token, that project will be unable to withdraw their funds, because the amount available will be less than the amount stated during deposit, and therefore the token's transfer() call will revert during withdrawal. For more users, consider what happens if the token has a 10% fee-on-transfer fee - deposits will be underfunded by 10%, and the projects trying to withdraw the last 10% of deposits/rewards will have their calls revert due to the contract not holding enough tokens. If a whale does a large withdrawal, the extra 10% that that whale gets will mean that many projects will not be able to withdraw anything at all.

Proof of Concept

Because the terminals rely on terminal stores, which only store the initial value provided during the payment, and provide it during distributions, the terminals are unable to use the decreased value when they later are told to distribute funds to a project.

JBSingleTokenPaymentTerminalStore.recordPaymentFrom() stores the value passed in:

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #1

372       // Add the amount to the token balance of the project.
373       balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] =
374         balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] +
375         _amount.value;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L372-L375

And provide that same value when recording a dispersion:

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #2

597       // Removed the distributed funds from the project's token balance.
598       balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] =
599         balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] -
600         distributedAmount;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L597-L600

The terminals themselves use the values directly, and don't consult their balances to look for changes (lines 817 and 850 below):

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #3

817       (JBFundingCycle memory _fundingCycle, uint256 _distributedAmount) = store.recordDistributionFor(
818         _projectId,
819         _amount,
820         _currency
821       );
822   
823       // The amount being distributed must be at least as much as was expected.
824       if (_distributedAmount < _minReturnedTokens) revert INADEQUATE_DISTRIBUTION_AMOUNT();
825   
826       // Get a reference to the project owner, which will receive tokens from paying the platform fee
827       // and receive any extra distributable funds not allocated to payout splits.
828       address payable _projectOwner = payable(projects.ownerOf(_projectId));
829   
830       // Define variables that will be needed outside the scoped section below.
831       // Keep a reference to the fee amount that was paid.
832       uint256 _fee;
833   
834       // Scoped section prevents stack too deep. `_feeDiscount`, `_feeEligibleDistributionAmount`, and `_leftoverDistributionAmount` only used within scope.
835       {
836         // Get the amount of discount that should be applied to any fees taken.
837         // If the fee is zero or if the fee is being used by an address that doesn't incur fees, set the discount to 100% for convinience.
838         uint256 _feeDiscount = fee == 0 || isFeelessAddress[msg.sender]
839           ? JBConstants.MAX_FEE_DISCOUNT
840           : _currentFeeDiscount(_projectId);
841   
842         // The amount distributed that is eligible for incurring fees.
843         uint256 _feeEligibleDistributionAmount;
844   
845         // The amount leftover after distributing to the splits.
846         uint256 _leftoverDistributionAmount;
847   
848         // Payout to splits and get a reference to the leftover transfer amount after all splits have been paid.
849         // Also get a reference to the amount that was distributed to splits from which fees should be taken.
850         (_leftoverDistributionAmount, _feeEligibleDistributionAmount) = _distributeToPayoutSplitsOf(
851           _projectId,
852           _fundingCycle.configuration,
853           payoutSplitsGroup,
854           _distributedAmount,
855           _feeDiscount
856         );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L817-L856

The terminals used the amounts stated, rather than transferred in (lines 349 and 356):

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #4

332     function pay(
333       uint256 _projectId,
334       uint256 _amount,
335       address _token,
336       address _beneficiary,
337       uint256 _minReturnedTokens,
338       bool _preferClaimedTokens,
339       string calldata _memo,
340       bytes calldata _metadata
341     ) external payable virtual override isTerminalOf(_projectId) returns (uint256) {
342       _token; // Prevents unused var compiler and natspec complaints.
343   
344       // ETH shouldn't be sent if this terminal's token isn't ETH.
345       if (token != JBTokens.ETH) {
346         if (msg.value > 0) revert NO_MSG_VALUE_ALLOWED();
347   
348         // Transfer tokens to this terminal from the msg sender.
349         _transferFrom(msg.sender, payable(address(this)), _amount);
350       }
351       // If this terminal's token is ETH, override _amount with msg.value.
352       else _amount = msg.value;
353   
354       return
355         _pay(
356           _amount,
357           msg.sender,
358           _projectId,
359           _beneficiary,
360           _minReturnedTokens,
361           _preferClaimedTokens,
362           _memo,
363           _metadata
364         );
365     }

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L332-L365

Same here (lines 555 and 561):

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #5

540     function addToBalanceOf(
541       uint256 _projectId,
542       uint256 _amount,
543       address _token,
544       string calldata _memo,
545       bytes calldata _metadata
546     ) external payable virtual override isTerminalOf(_projectId) {
547       _token; // Prevents unused var compiler and natspec complaints.
548   
549       // If this terminal's token isn't ETH, make sure no msg.value was sent, then transfer the tokens in from msg.sender.
550       if (token != JBTokens.ETH) {
551         // Amount must be greater than 0.
552         if (msg.value > 0) revert NO_MSG_VALUE_ALLOWED();
553   
554         // Transfer tokens to this terminal from the msg sender.
555         _transferFrom(msg.sender, payable(address(this)), _amount);
556       }
557       // If the terminal's token is ETH, override `_amount` with msg.value.
558       else _amount = msg.value;
559   
560       // Add to balance while only refunding held fees if the funds aren't originating from a feeless terminal.
561       _addToBalanceOf(_projectId, _amount, !isFeelessAddress[msg.sender], _memo, _metadata);
562     }

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L540-L562

The transfer of fees and reserves have the same issue.

Tools Used

Code inspection

Measure the contract balance before and after the call to transfer()/transferFrom() in JBERC20PaymentTerminal._transferFrom(), and use the difference between the two as the amount, rather than the amount stated

Findings Information

🌟 Selected for report: oyc_109

Also found by: 0x52, IllIllI, pashov

Labels

bug
duplicate
2 (Med Risk)
old-submission-method
valid

Awards

781.4991 USDC - $781.50

External Links

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L594-L603 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L540

Vulnerability details

Impact

If too many fees are received, there may be so many entries in the array that tracks them that the function that withdraws the fees will run out of gas. In addition, since addToBalanceOf() also passes through the fee array, the project will be unable to receive any further funds.

Proof of Concept

JBPayoutRedemptionPaymentTerminal.processFees() asks for the full fees array, and attempts to process each one. Depending on how much gas each fee processing overhead there is (e.g. how expensive the token transfers are in _processFee()), the function may revert:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #1

573     function processFees(uint256 _projectId)
574       external
575       virtual
576       override
577       requirePermissionAllowingOverride(
578         projects.ownerOf(_projectId),
579         _projectId,
580         JBOperations.PROCESS_FEES,
581         msg.sender == owner()
582       )
583     {
584       // Get a reference to the project's held fees.
585       JBFee[] memory _heldFees = _heldFeesOf[_projectId];
586   
587       // Delete the held fees.
588       delete _heldFeesOf[_projectId];
589   
590       // Push array length in stack
591       uint256 _heldFeeLength = _heldFees.length;
592   
593       // Process each fee.
594       for (uint256 _i = 0; _i < _heldFeeLength; ) {
595         // Get the fee amount.
596         uint256 _amount = _feeAmount(
597           _heldFees[_i].amount,
598           _heldFees[_i].fee,
599           _heldFees[_i].feeDiscount
600         );
601   
602         // Process the fee.
603         _processFee(_amount, _heldFees[_i].beneficiary);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L573-L603

Separately, JBPayoutRedemptionPaymentTerminal.addToBalanceOf() is used by the terminal to receieve funds for a specific project:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #2

530     /**
531       @notice
532       Receives funds belonging to the specified project.
533   
534       @param _projectId The ID of the project to which the funds received belong.
535       @param _amount The amount of tokens to add, as a fixed point number with the same number of decimals as this terminal. If this is an ETH terminal, this is ignored and msg.value is used instead.
536       @param _token The token being paid. This terminal ignores this property since it only manages one currency. 
537       @param _memo A memo to pass along to the emitted event.
538       @param _metadata Extra data to pass along to the emitted event.
539     */
540     function addToBalanceOf(
541       uint256 _projectId,
542       uint256 _amount,
543       address _token,
544       string calldata _memo,
545       bytes calldata _metadata
546     ) external payable virtual override isTerminalOf(_projectId) {
547       _token; // Prevents unused var compiler and natspec complaints.
548   
549       // If this terminal's token isn't ETH, make sure no msg.value was sent, then transfer the tokens in from msg.sender.
550       if (token != JBTokens.ETH) {
551         // Amount must be greater than 0.
552         if (msg.value > 0) revert NO_MSG_VALUE_ALLOWED();
553   
554         // Transfer tokens to this terminal from the msg sender.
555         _transferFrom(msg.sender, payable(address(this)), _amount);
556       }
557       // If the terminal's token is ETH, override `_amount` with msg.value.
558       else _amount = msg.value;
559   
560       // Add to balance while only refunding held fees if the funds aren't originating from a feeless terminal.
561       _addToBalanceOf(_projectId, _amount, !isFeelessAddress[msg.sender], _memo, _metadata);
562     }

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L530-L562

It calls _addToBalanceOf(), which if the project supports fees, first processes fees:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #3

1354     function _addToBalanceOf(
1355       uint256 _projectId,
1356       uint256 _amount,
1357       bool _shouldRefundHeldFees,
1358       string memory _memo,
1359       bytes memory _metadata
1360     ) internal {
1361       // Refund any held fees to make sure the project doesn't pay double for funds going in and out of the protocol.
1362       uint256 _refundedFees = _shouldRefundHeldFees ? _refundHeldFees(_projectId, _amount) : 0;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L1354-L1362

The processing involves the canceling out of fees based on the total amount passed in. While there are no external calls in this processing, if the amount is large enough, and there are dust amounts for each fee, and there are many of these fees, the operation can run out of gas, preventing any further deposits of similar or larger size:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #4

1379     function _refundHeldFees(uint256 _projectId, uint256 _amount)
1380       internal
1381       returns (uint256 refundedFees)
1382     {
1383       // Get a reference to the project's held fees.
1384       JBFee[] memory _heldFees = _heldFeesOf[_projectId];
1385   
1386       // Delete the current held fees.
1387       delete _heldFeesOf[_projectId];
1388   
1389       // Get a reference to the leftover amount once all fees have been settled.
1390       uint256 leftoverAmount = _amount;
1391   
1392       // Push length in stack
1393       uint256 _heldFeesLength = _heldFees.length;
1394   
1395       // Process each fee.
1396       for (uint256 _i = 0; _i < _heldFeesLength; ) {
1397         if (leftoverAmount == 0) _heldFeesOf[_projectId].push(_heldFees[_i]);
1398         else if (leftoverAmount >= _heldFees[_i].amount) {
1399           unchecked {
1400             leftoverAmount = leftoverAmount - _heldFees[_i].amount;
1401             refundedFees += _feeAmount(
1402               _heldFees[_i].amount,
1403               _heldFees[_i].fee,
1404               _heldFees[_i].feeDiscount
1405             );
1406           }
1407         } else {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L1379-L1407

Tools Used

Code inspection

Allow processFees() to batch its operations in smaller chunks, and in addToBalanceOf() have a maximum number of fee entries that are processed, after which the reconciliation is deferred until the next call to addToBalanceOf()

#0 - drgorillamd

2022-07-12T20:02:46Z

Duplicate of #348

Summary

Low Risk Issues

IssueInstances
1Weight of one being used as zero not documented1
2Calls may run out of gas until arrays are reduced in size2
3Dust amounts not compensated, even if not using price oracle1
4Splits can't be locked once the timestamp passes type(uint48).max1
5Unsafe use of transfer()/transferFrom() with IERC202

Total: 7 instances over 5 issues

Non-critical Issues

IssueInstances
1Confusing variable names1
2Return values of approve() not checked1
3Adding a return statement when the function defines a named return variable, is redundant4
4Non-assembly method available1
5constants should be defined rather than using magic numbers37
6Use a more recent version of solidity1
7Use a more recent version of solidity3
8Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)1
9Constant redefined elsewhere11
10Inconsistent spacing in comments1
11Lines are too long49
12Typos17
13File is missing NatSpec29
14NatSpec is incomplete5
15Event is missing indexed fields34
16Not using the named return variables anywhere in the function is confusing6

Total: 201 instances over 16 issues

Low Risk Issues

1. Weight of one being used as zero not documented

The comments and code below say that a weight of one is being used as a weight of zero. If a project is mature, or eventually becomes mature, a weight of one may in fact be a useful weighting, and the project owners will become very confused when they are unable to receive funds with this weighting

There is 1 instance of this issue:

File: contracts/JBFundingCycleStore.sol   #1

467        // A weight of 1 is treated as a weight of 0.
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)
471:         : _deriveWeightFrom(_baseFundingCycle, _start);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L467-L471

2. Calls may run out of gas until arrays are reduced in size

The examples below are of functions that may revert due to the size of the data they're processing, but no funds are at risk because the arrays can be changed

There are 2 instances of this issue:

File: contracts/JBDirectory.sol   #1

357:     if (isTerminalOf(_projectId, _terminal)) return;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L357

File: contracts/JBDirectory.sol   #2

141:       if (_terminal.acceptsToken(_token, _projectId)) return _terminal;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L141

3. Dust amounts not compensated, even if not using price oracle

If there's a fixed weighting between what the user provides, and what is minted for them, there should be code that tracks partial token amounts, so that later payments are compensated for their prior partial amounts

There is 1 instance of this issue:

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #1

385      uint256 _weightRatio = _amount.currency == _baseWeightCurrency
386        ? 10**_decimals
387        : prices.priceFor(_amount.currency, _baseWeightCurrency, _decimals);
388  
389      // Find the number of tokens to mint, as a fixed point number with as many decimals as `weight` has.
390:     tokenCount = PRBMath.mulDiv(_amount.value, _weight, _weightRatio);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L385-L390

4. Splits can't be locked once the timestamp passes type(uint48).max

This behavior isn't documented anywhere, and a project will be confused by this behavior when that time comes (the original developers will be unable to explain it because they'll be dead)

There is 1 instance of this issue:

File: contracts/JBSplitsStore.sol   #1

261:         if (_splits[_i].lockedUntil > type(uint48).max) revert INVALID_LOCKED_UNTIL();

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L261

5. Unsafe use of transfer()/transferFrom() with IERC20

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions on L1 do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert (see this link for a test case). Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

There are 2 instances of this issue:

File: contracts/JBERC20PaymentTerminal.sol   #1

87:         ? IERC20(token).transfer(_to, _amount)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L87

File: contracts/JBERC20PaymentTerminal.sol   #2

88:         : IERC20(token).transferFrom(_from, _to, _amount);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L88

Non-critical Issues

1. Confusing variable names

It was well into my review before I realized that 'configuration' means the timestamp at which the configuration is set, not the actual configuration details. It would be helpful to people reading the code to name it something like configTimestamp in all places. Below is one example of many

There is 1 instance of this issue:

File: contracts/JBFundingCycleStore.sol   #1

332:     uint256 _configuration = block.timestamp;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L332

2. Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

There is 1 instance of this issue:

File: contracts/JBERC20PaymentTerminal.sol   #1

99:       IERC20(token).approve(_to, _amount);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L99

3. Adding a return statement when the function defines a named return variable, is redundant

There are 4 instances of this issue:

File: contracts/JBFundingCycleStore.sol   #1

152:        if (_isApproved(_projectId, fundingCycle)) return fundingCycle;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L152

File: contracts/JBFundingCycleStore.sol   #2

553:      if (_fundingCycle.number == 1) return configuration;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L553

File: contracts/JBFundingCycleStore.sol   #3

716:      if (_baseFundingCycle.discountRate == 0) return weight;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L716

File: contracts/JBFundingCycleStore.sol   #4

835:      if (_configuration == 0) return fundingCycle;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L835

4. Non-assembly method available

assembly{ id := chainid() } => uint256 id = block.chainid, assembly { size := extcodesize() } => uint256 size = address().code.length

There is 1 instance of this issue:

File: contracts/JBFundingCycleStore.sol   #1

320:          _size := extcodesize(_ballot) // No contract at the address ?

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L320

5. constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 37 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit 18
209:      uint256 _adjustedOverflow = (decimals == 18)

/// @audit 18
211:        : JBFixedPointNumber.adjustDecimals(_overflow, decimals, 18);

/// @audit 32
1074:             bytes memory _projectMetadata = new bytes(32);

/// @audit 32
1113:             bytes memory _projectMetadata = new bytes(32);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L209

File: contracts/JBController.sol

/// @audit 232
166:      return (uint256(uint232(_data)), _data >> 232);

/// @audit 232
194:      return (uint256(uint232(_data)), _data >> 232);

/// @audit 18
948:                18,

/// @audit 232
1037:           (_constraints.distributionLimitCurrency << 232);

/// @audit 232
1045:           (_constraints.overflowAllowanceCurrency << 232);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L166

File: contracts/JBDirectory.sol

/// @audit 8
232:        !uint8(_fundingCycle.metadata >> 8).setControllerAllowed()

/// @audit 8
267:        !uint8(_fundingCycle.metadata >> 8).setTerminalsAllowed()

/// @audit 8
365:        !uint8(_fundingCycle.metadata >> 8).setTerminalsAllowed()

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L232

File: contracts/JBETHPaymentTerminal.sol

/// @audit 18
42:         18, // 18 decimals.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBETHPaymentTerminal.sol#L42

File: contracts/JBFundingCycleStore.sol

/// @audit 160
354:        packed |= _data.duration << 160;

/// @audit 224
357:        packed |= _data.discountRate << 224;

/// @audit 88
516:      packed |= _basedOn << 88;

/// @audit 144
519:      packed |= _start << 144;

/// @audit 200
522:      packed |= _number << 200;

/// @audit 88
844:      fundingCycle.basedOn = uint256(uint56(_packedIntrinsicProperties >> 88));

/// @audit 144
846:      fundingCycle.start = uint256(uint56(_packedIntrinsicProperties >> 144));

/// @audit 200
848:      fundingCycle.number = uint256(uint56(_packedIntrinsicProperties >> 200));

/// @audit 160
855:      fundingCycle.duration = uint256(uint64(_packedUserProperties >> 160));

/// @audit 224
857:      fundingCycle.discountRate = uint256(uint32(_packedUserProperties >> 224));

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L354

File: contracts/JBOperatorStore.sol

/// @audit 255
63:       if (_permissionIndex > 255) revert PERMISSION_INDEX_OUT_OF_BOUNDS();

/// @audit 255
88:         if (_permissionIndex > 255) revert PERMISSION_INDEX_OUT_OF_BOUNDS();

/// @audit 255
168:        if (_index > 255) revert PERMISSION_INDEX_OUT_OF_BOUNDS();

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L63

File: contracts/JBSingleTokenPaymentTerminalStore.sol

/// @audit 18
868:        : PRBMath.mulDiv(_ethOverflow, 10**18, prices.priceFor(JBCurrencies.ETH, _currency, 18));

/// @audit 18
868:        : PRBMath.mulDiv(_ethOverflow, 10**18, prices.priceFor(JBCurrencies.ETH, _currency, 18));

/// @audit 18
872:        (_decimals == 18)

/// @audit 18
874:          : JBFixedPointNumber.adjustDecimals(_totalOverflow18Decimal, 18, _decimals);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L868

File: contracts/JBSplitsStore.sol

/// @audit 34
251:        _packedSplitParts1 |= _splits[_i].projectId << 34;

/// @audit 90
253:        _packedSplitParts1 |= uint256(uint160(address(_splits[_i].beneficiary))) << 90;

/// @audit 48
266:          _packedSplitParts2 |= uint256(uint160(address(_splits[_i].allocator))) << 48;

/// @audit 34
318:        _split.projectId = uint256(uint56(_packedSplitPart1 >> 34));

/// @audit 90
320:        _split.beneficiary = payable(address(uint160(_packedSplitPart1 >> 90)));

/// @audit 48
330:          _split.allocator = IJBSplitAllocator(address(uint160(_packedSplitPart2 >> 48)));

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L251

File: contracts/JBTokenStore.sol

/// @audit 18
249:      if (_token != IJBToken(address(0)) && _token.decimals() != 18)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L249

6. Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

There is 1 instance of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #1

2:    pragma solidity 0.8.6;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L2

7. Use a more recent version of solidity

Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

There are 3 instances of this issue:

File: contracts/JBController.sol   #1

2:    pragma solidity 0.8.6;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L2

File: contracts/JBDirectory.sol   #2

2:    pragma solidity 0.8.6;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L2

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #3

2:    pragma solidity 0.8.6;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L2

8. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

There is 1 instance of this issue:

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #1

868:        : PRBMath.mulDiv(_ethOverflow, 10**18, prices.priceFor(JBCurrencies.ETH, _currency, 18));

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L868

9. Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

There are 11 instances of this issue:

File: contracts/JBController.sol

/// @audit seen in contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol 
111:    IJBProjects public immutable override projects;

/// @audit seen in contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol 
129:    IJBSplitsStore public immutable override splitsStore;

/// @audit seen in contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol 
135:    IJBDirectory public immutable override directory;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L111

File: contracts/JBDirectory.sol

/// @audit seen in contracts/JBController.sol 
65:     IJBProjects public immutable override projects;

/// @audit seen in contracts/JBController.sol 
71:     IJBFundingCycleStore public immutable override fundingCycleStore;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L65

File: contracts/JBSingleTokenPaymentTerminalStore.sol

/// @audit seen in contracts/JBController.sol 
61:     IJBDirectory public immutable override directory;

/// @audit seen in contracts/JBDirectory.sol 
67:     IJBFundingCycleStore public immutable override fundingCycleStore;

/// @audit seen in contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol 
73:     IJBPrices public immutable override prices;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L61

File: contracts/JBSplitsStore.sol

/// @audit seen in contracts/JBDirectory.sol 
81:     IJBProjects public immutable override projects;

/// @audit seen in contracts/JBSingleTokenPaymentTerminalStore.sol 
87:     IJBDirectory public immutable override directory;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L81

File: contracts/JBTokenStore.sol

/// @audit seen in contracts/JBSplitsStore.sol 
56:     IJBProjects public immutable override projects;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L56

10. Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There is 1 instance of this issue:

File: contracts/JBController.sol   #1

912:      //Transfer between all splits.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L912

11. Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length

There are 49 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

26:     A project can transfer its funds, along with the power to reconfigure and mint/burn their tokens, from this contract to another allowed terminal of the same token type contract at any time.

30:     IJBPayoutRedemptionPaymentTerminal: General interface for the methods in this contract that interact with the blockchain's state according to the protocol's rules.

322:      @param _amount The amount of terminal tokens being received, as a fixed point number with the same amount of decimals as this terminal. If this terminal's token is ETH, this is ignored and msg.value is used in its place.

326:      @param _preferClaimedTokens A flag indicating whether the request prefers to mint project tokens into the beneficiaries wallet rather than leaving them unclaimed. This is only possible if the project has an attached token contract. Leaving them unclaimed saves gas.

327:      @param _memo A memo to pass along to the emitted event, and passed along the the funding cycle's data source and delegate.  A data source can alter the memo before emitting in the event and forwarding to the delegate.

423:      Anyone can distribute payouts on a project's behalf. The project can preconfigure a wildcard split that is used to send funds to msg.sender. This can be used to incentivize calling this function.

432:      @param _minReturnedTokens The minimum number of terminal tokens that the `_amount` should be valued at in terms of this terminal's currency, as a fixed point number with the same number of decimals as this terminal.

461:      @param _amount The amount of terminal tokens to use from this project's current allowance, as a fixed point number with the same amount of decimals as this terminal.

464:      @param _minReturnedTokens The minimum number of tokens that the `_amount` should be valued at in terms of this terminal's currency, as a fixed point number with 18 decimals.

468:      @return netDistributedAmount The amount of tokens that was distributed to the beneficiary, as a fixed point number with the same amount of decimals as the terminal.

535:      @param _amount The amount of tokens to add, as a fixed point number with the same number of decimals as this terminal. If this is an ETH terminal, this is ignored and msg.value is used instead.

796:      Anyone can distribute payouts on a project's behalf. The project can preconfigure a wildcard split that is used to send funds to msg.sender. This can be used to incentivize calling this function.

804:      @param _minReturnedTokens The minimum number of terminal tokens that the `_amount` should be valued at in terms of this terminal's currency, as a fixed point number with the same number of decimals as this terminal.

913:      @param _amount The amount of terminal tokens to use from this project's current allowance, as a fixed point number with the same amount of decimals as this terminal.

915:      @param _minReturnedTokens The minimum number of tokens that the `_amount` should be valued at in terms of this terminal's currency, as a fixed point number with 18 decimals.

919:      @return netDistributedAmount The amount of tokens that was distributed to the beneficiary, as a fixed point number with the same amount of decimals as the terminal.

1249:     @param _amount The amount of terminal tokens being received, as a fixed point number with the same amount of decimals as this terminal. If this terminal's token is ETH, this is ignored and msg.value is used in its place.

1254:     @param _preferClaimedTokens A flag indicating whether the request prefers to mint project tokens into the beneficiaries wallet rather than leaving them unclaimed. This is only possible if the project has an attached token contract. Leaving them unclaimed saves gas.

1255:     @param _memo A memo to pass along to the emitted event, and passed along the the funding cycle's data source and delegate.  A data source can alter the memo before emitting in the event and forwarding to the delegate.

1349:     @param _amount The amount of tokens to add, as a fixed point number with the same number of decimals as this terminal. If this is an ETH terminal, this is ignored and msg.value is used instead.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L26

File: contracts/JBController.sol

23:     IJBController: General interface for the generic controller methods in this contract that interacts with funding cycles and tokens according to the protocol's rules.

28:     JBOperatable: Several functions in this contract can only be accessed by a project owner, or an address that has been preconfifigured to be an operator of the project.

61:       The difference between the processed token tracker of a project and the project's token's total supply is the amount of tokens that still need to have reserves minted against them.

401:      @param _metadata Metadata specifying the controller specific params that a funding cycle can have. These properties will remain fixed for the duration of the funding cycle.

404:      @param _fundAccessConstraints An array containing amounts that a project can use from its treasury for each payment terminal. Amounts are fixed point numbers using the same number of decimals as the accompanying terminal. The `_distributionLimit` and `_overflowAllowance` parameters must fit in a `uint232`.

455:      @param _metadata Metadata specifying the controller specific params that a funding cycle can have. These properties will remain fixed for the duration of the funding cycle.

458:      @param _fundAccessConstraints An array containing amounts that a project can use from its treasury for each payment terminal. Amounts are fixed point numbers using the same number of decimals as the accompanying terminal. The `_distributionLimit` and `_overflowAllowance` parameters must fit in a `uint232`.

505:      Proposes a configuration of a subsequent funding cycle that will take effect once the current one expires if it is approved by the current funding cycle's ballot.

512:      @param _metadata Metadata specifying the controller specific params that a funding cycle can have. These properties will remain fixed for the duration of the funding cycle.

515:      @param _fundAccessConstraints An array containing amounts that a project can use from its treasury for each payment terminal. Amounts are fixed point numbers using the same number of decimals as the accompanying terminal. The `_distributionLimit` and `_overflowAllowance` parameters must fit in a `uint232`.

976:      @param _metadata Metadata specifying the controller specific params that a funding cycle can have. These properties will remain fixed for the duration of the funding cycle.

979:      @param _fundAccessConstraints An array containing amounts that a project can use from its treasury for each payment terminal. Amounts are fixed point numbers using the same number of decimals as the accompanying terminal.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L23

File: contracts/JBDirectory.sol

12:     Keeps a reference of which terminal contracts each project is currently accepting funds through, and which controller contract is managing each project's tokens and funding cycles.

228:      // Setting controller is allowed if called from the current controller, or if the project doesn't have a current controller, or if the project's funding cycle allows setting the controller. Revert otherwise.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L12

File: contracts/JBFundingCycleStore.sol

18:     JBControllerUtility: Includes convenience functionality for checking if the message sender is the current controller of the project whose data is being manipulated.

229:        // If it's not approved or if it hasn't yet started, get a reference to the funding cycle that the latest is based on, which has the latest approved configuration.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L18

File: contracts/JBOperatorStore.sol

34:       Permissions are stored in a packed `uint256`. Each 256 bits represents the on/off state of a permission. Applications can specify the significance of each index.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L34

File: contracts/JBSingleTokenPaymentTerminalStore.sol

19:     IJBSingleTokenPaymentTerminalStore: General interface for the methods in this contract that interact with the blockchain's state according to the protocol's rules.

93:       The amount of funds that a project has distributed from its limit during the current funding cycle for each terminal, in terms of the distribution limit's currency.

111:      The amount of funds that a project has used from its allowance during the current funding cycle configuration for each terminal, in terms of the overflow allowance's currency.

179:      The current amount of overflowed tokens from a terminal that can be reclaimed by the specified number of tokens, using the total token supply and overflow in the ecosystem.

193:      @param _useTotalOverflow A flag indicating whether the overflow used in the calculation should be summed from all of the project's terminals. If false, overflow should be limited to the amount in the specified `_terminal`.

235:      The current amount of overflowed tokens from a terminal that can be reclaimed by the specified number of tokens, using the specified total token supply and overflow amounts.

295:      Mint's the project's tokens according to values provided by a configured data source. If no data source is configured, mints tokens proportional to the amount of the contribution.

398:      Redeems the project's tokens according to values provided by a configured data source. If no data source is configured, redeems tokens along a redemption bonding curve that is a function of the number of tokens being burned.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L19

File: contracts/JBTokenStore.sol

30:     JBControllerUtility: Includes convenience functionality for checking if the message sender is the current controller of the project whose data is being manipulated.

231:      @param _token The new token. Send an empty address to remove the project's current token without adding a new one, if claiming tokens isn't currency required by the project

281:      @param _preferClaimedTokens A flag indicating whether there's a preference for minted tokens to be claimed automatically into the `_holder`s wallet if the project currently has a token contract attached.

318:      @param _preferClaimedTokens A flag indicating whether there's a preference for tokens to burned from the `_holder`s wallet if the project currently has a token contract attached.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L30

12. Typos

There are 17 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit adherance
247:      @param _interfaceId The ID of the interface to check for adherance to.

/// @audit incure
426:      All funds distributed outside of this contract or any feeless terminals incure the protocol fee.

/// @audit incure
799:      All funds distributed outside of this contract or any feeless terminals incure the protocol fee.

/// @audit convinience
837:        // If the fee is zero or if the fee is being used by an address that doesn't incur fees, set the discount to 100% for convinience.

/// @audit convinience
948:        // If the fee is zero or if the fee is being used by an address that doesn't incur fees, set the discount to 100% for convinience.

/// @audit prefered
1077:             // Add to balance if prefered.

/// @audit prefered
1116:             // Add to balance if prefered.

/// @audit guage
1470:       // If the guage reverts, set the discount to 0.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L247

File: contracts/interfaces/IJBSplitAllocator.sol

/// @audit transfered
27:       Critical business logic should be protected by an appropriate access control. The token and/or eth are optimistically transfered

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitAllocator.sol#L27

File: contracts/JBController.sol

/// @audit preconfifigured
28:     JBOperatable: Several functions in this contract can only be accessed by a project owner, or an address that has been preconfifigured to be an operator of the project.

/// @audit adherance
341:      @param _interfaceId The ID of the interface to check for adherance to.

/// @audit dont
898:      @return leftoverAmount If the splits percents dont add up to 100%, the leftover amount is returned.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L28

File: contracts/JBFundingCycleStore.sol

/// @audit instrinsic
47:       _projectId The ID of the project to get instrinsic properties of.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L47

File: contracts/JBProjects.sol

/// @audit adherance
84:       @param _interfaceId The ID of the interface to check for adherance to.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBProjects.sol#L84

File: contracts/JBSingleTokenPaymentTerminalStore.sol

/// @audit mumber
384:      // The weight is always a fixed point mumber with 18 decimals. To ensure this, the ratio should use the same number of decimals as the `_amount`.

/// @audit areference
452:          // Get areference to the terminal's currency.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L384

File: contracts/JBSplitsStore.sol

/// @audit extention
218:            // Allow lock extention.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L218

13. File is missing NatSpec

There are 29 instances of this issue:

File: contracts/interfaces/IJBAllowanceTerminal.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBAllowanceTerminal.sol

File: contracts/interfaces/IJBController.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBController.sol

File: contracts/interfaces/IJBControllerUtility.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBControllerUtility.sol

File: contracts/interfaces/IJBDirectory.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBDirectory.sol

File: contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol

File: contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol

File: contracts/interfaces/IJBFeeGauge.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFeeGauge.sol

File: contracts/interfaces/IJBFundingCycleBallot.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleBallot.sol

File: contracts/interfaces/IJBFundingCycleStore.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleStore.sol

File: contracts/interfaces/IJBMigratable.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBMigratable.sol

File: contracts/interfaces/IJBOperatable.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBOperatable.sol

File: contracts/interfaces/IJBOperatorStore.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBOperatorStore.sol

File: contracts/interfaces/IJBPaymentTerminal.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPaymentTerminal.sol

File: contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol

File: contracts/interfaces/IJBPayoutTerminal.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutTerminal.sol

File: contracts/interfaces/IJBPriceFeed.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPriceFeed.sol

File: contracts/interfaces/IJBPrices.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPrices.sol

File: contracts/interfaces/IJBProjectPayer.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjectPayer.sol

File: contracts/interfaces/IJBProjects.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjects.sol

File: contracts/interfaces/IJBReconfigurationBufferBallot.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBReconfigurationBufferBallot.sol

File: contracts/interfaces/IJBRedemptionTerminal.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBRedemptionTerminal.sol

File: contracts/interfaces/IJBSingleTokenPaymentTerminal.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminal.sol

File: contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol

File: contracts/interfaces/IJBSplitsPayer.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsPayer.sol

File: contracts/interfaces/IJBSplitsStore.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsStore.sol

File: contracts/interfaces/IJBTerminalUtility.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTerminalUtility.sol

File: contracts/interfaces/IJBToken.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBToken.sol

File: contracts/interfaces/IJBTokenStore.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenStore.sol

File: contracts/interfaces/IJBTokenUriResolver.sol

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenUriResolver.sol

14. NatSpec is incomplete

There are 5 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit Missing: '@return'
247       @param _interfaceId The ID of the interface to check for adherance to.
248     */
249     function supportsInterface(bytes4 _interfaceId)
250       public
251       view
252       virtual
253       override(JBSingleTokenPaymentTerminal, IERC165)
254:      returns (bool)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L247-L254

File: contracts/JBController.sol

/// @audit Missing: '@param _configuration'
249     /** 
250       @notice
251       A project's funding cycle for the specified configuration along with its metadata.
252   
253       @param _projectId The ID of the project to which the funding cycle belongs.
254     
255       @return fundingCycle The funding cycle.
256       @return metadata The funding cycle's metadata.
257     */
258     function getFundingCycleOf(uint256 _projectId, uint256 _configuration)
259       external
260       view
261       override
262:      returns (JBFundingCycle memory fundingCycle, JBFundingCycleMetadata memory metadata)

/// @audit Missing: '@return'
341       @param _interfaceId The ID of the interface to check for adherance to.
342     */
343     function supportsInterface(bytes4 _interfaceId)
344       public
345       view
346       virtual
347       override(ERC165, IERC165)
348:      returns (bool)

/// @audit Missing: '@return'
560       @param _symbol The ERC20's symbol.
561     */
562     function issueTokenFor(
563       uint256 _projectId,
564       string calldata _name,
565       string calldata _symbol
566     )
567       external
568       virtual
569       override
570       requirePermission(projects.ownerOf(_projectId), _projectId, JBOperations.ISSUE)
571:      returns (IJBToken token)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L249-L262

File: contracts/JBProjects.sol

/// @audit Missing: '@return'
84        @param _interfaceId The ID of the interface to check for adherance to.
85      */
86      function supportsInterface(bytes4 _interfaceId)
87        public
88        view
89        virtual
90        override(IERC165, ERC721)
91:       returns (bool)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBProjects.sol#L84-L91

15. Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (threefields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question

There are 34 instances of this issue:

File: contracts/interfaces/IJBController.sol

19:     event LaunchProject(uint256 configuration, uint256 projectId, string memo, address caller);

21:     event LaunchFundingCycles(uint256 configuration, uint256 projectId, string memo, address caller);

23      event ReconfigureFundingCycles(
24        uint256 configuration,
25        uint256 projectId,
26        string memo,
27        address caller
28:     );

58      event MintTokens(
59        address indexed beneficiary,
60        uint256 indexed projectId,
61        uint256 tokenCount,
62        uint256 beneficiaryTokenCount,
63        string memo,
64        uint256 reservedRate,
65        address caller
66:     );

68      event BurnTokens(
69        address indexed holder,
70        uint256 indexed projectId,
71        uint256 tokenCount,
72        string memo,
73        address caller
74:     );

76:     event Migrate(uint256 indexed projectId, IJBMigratable to, address caller);

78:     event PrepMigration(uint256 indexed projectId, address from, address caller);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBController.sol#L19

File: contracts/interfaces/IJBDirectory.sol

9:      event SetController(uint256 indexed projectId, address indexed controller, address caller);

11:     event AddTerminal(uint256 indexed projectId, IJBPaymentTerminal indexed terminal, address caller);

13:     event SetTerminals(uint256 indexed projectId, IJBPaymentTerminal[] terminals, address caller);

22:     event SetIsAllowedToSetFirstController(address indexed addr, bool indexed flag, address caller);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBDirectory.sol#L9

File: contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol

8       event DeployProjectPayer(
9         IJBProjectPayer indexed projectPayer,
10        uint256 defaultProjectId,
11        address defaultBeneficiary,
12        bool defaultPreferClaimedTokens,
13        string defaultMemo,
14        bytes defaultMetadata,
15        bool preferAddToBalance,
16        IJBDirectory directory,
17        address owner,
18        address caller
19:     );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol#L8-L19

File: contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol

8       event DeploySplitsPayer(
9         IJBSplitsPayer indexed splitsPayer,
10        uint256 defaultSplitsProjectId,
11        uint256 defaultSplitsDomain,
12        uint256 defaultSplitsGroup,
13        IJBSplitsStore splitsStore,
14        uint256 defaultProjectId,
15        address defaultBeneficiary,
16        bool defaultPreferClaimedTokens,
17        string defaultMemo,
18        bytes defaultMetadata,
19        bool preferAddToBalance,
20        address owner,
21        address caller
22:     );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol#L8-L22

File: contracts/interfaces/IJBFundingCycleStore.sol

9       event Configure(
10        uint256 indexed configuration,
11        uint256 indexed projectId,
12        JBFundingCycleData data,
13        uint256 metadata,
14        uint256 mustStartAtOrAfter,
15        address caller
16:     );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleStore.sol#L9-L16

File: contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol

26      event AddToBalance(
27        uint256 indexed projectId,
28        uint256 amount,
29        uint256 refundedFees,
30        string memo,
31        bytes metadata,
32        address caller
33:     );

35      event Migrate(
36        uint256 indexed projectId,
37        IJBPaymentTerminal indexed to,
38        uint256 amount,
39        address caller
40:     );

105:    event DelegateDidPay(IJBPayDelegate indexed delegate, JBDidPayData data, address caller);

120     event DelegateDidRedeem(
121       IJBRedemptionDelegate indexed delegate,
122       JBDidRedeemData data,
123       address caller
124:    );

135:    event SetFee(uint256 fee, address caller);

137:    event SetFeeGauge(IJBFeeGauge indexed feeGauge, address caller);

139:    event SetFeelessAddress(address indexed addrs, bool indexed flag, address caller);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol#L26-L33

File: contracts/interfaces/IJBPrices.sol

7:      event AddFeed(uint256 indexed currency, uint256 indexed base, IJBPriceFeed feed);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPrices.sol#L7

File: contracts/interfaces/IJBProjectPayer.sol

8       event SetDefaultValues(
9         uint256 indexed projectId,
10        address indexed beneficiary,
11        bool preferClaimedTokens,
12        string memo,
13        bytes metadata,
14        bool preferAddToBalance,
15        address caller
16:     );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjectPayer.sol#L8-L16

File: contracts/interfaces/IJBProjects.sol

9       event Create(
10        uint256 indexed projectId,
11        address indexed owner,
12        JBProjectMetadata metadata,
13        address caller
14:     );

16:     event SetMetadata(uint256 indexed projectId, JBProjectMetadata metadata, address caller);

18:     event SetTokenUriResolver(IJBTokenUriResolver indexed resolver, address caller);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjects.sol#L9-L14

File: contracts/interfaces/IJBSplitsPayer.sol

15      event Pay(
16        uint256 indexed projectId,
17        address beneficiary,
18        address token,
19        uint256 amount,
20        uint256 decimals,
21        uint256 leftoverAmount,
22        uint256 minReturnedTokens,
23        bool preferClaimedTokens,
24        string memo,
25        bytes metadata,
26        address caller
27:     );

29      event AddToBalance(
30        uint256 indexed projectId,
31        address beneficiary,
32        address token,
33        uint256 amount,
34        uint256 decimals,
35        uint256 leftoverAmount,
36        string memo,
37        bytes metadata,
38        address caller
39:     );

48      event DistributeToSplit(
49        JBSplit split,
50        uint256 amount,
51        address defaultBeneficiary,
52        address caller
53:     );

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsPayer.sol#L15-L27

File: contracts/interfaces/IJBTokenStore.sol

8       event Issue(
9         uint256 indexed projectId,
10        IJBToken indexed token,
11        string name,
12        string symbol,
13        address caller
14:     );

16      event Mint(
17        address indexed holder,
18        uint256 indexed projectId,
19        uint256 amount,
20        bool tokensWereClaimed,
21        bool preferClaimedTokens,
22        address caller
23:     );

25      event Burn(
26        address indexed holder,
27        uint256 indexed projectId,
28        uint256 amount,
29        uint256 initialUnclaimedBalance,
30        uint256 initialClaimedBalance,
31        bool preferClaimedTokens,
32        address caller
33:     );

35      event Claim(
36        address indexed holder,
37        uint256 indexed projectId,
38        uint256 initialUnclaimedBalance,
39        uint256 amount,
40        address caller
41:     );

43:     event ShouldRequireClaim(uint256 indexed projectId, bool indexed flag, address caller);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenStore.sol#L8-L14

16. Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 6 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit reclaimAmount
385     function redeemTokensOf(
386       address _holder,
387       uint256 _projectId,
388       uint256 _tokenCount,
389       address _token,
390       uint256 _minReturnedTokens,
391       address payable _beneficiary,
392       string memory _memo,
393       bytes memory _metadata
394     )
395       external
396       virtual
397       override
398       requirePermission(_holder, _projectId, JBOperations.REDEEM)
399:      returns (uint256 reclaimAmount)

/// @audit netLeftoverDistributionAmount
437     function distributePayoutsOf(
438       uint256 _projectId,
439       uint256 _amount,
440       uint256 _currency,
441       address _token,
442       uint256 _minReturnedTokens,
443       string calldata _memo
444:    ) external virtual override returns (uint256 netLeftoverDistributionAmount) {

/// @audit netDistributedAmount
470     function useAllowanceOf(
471       uint256 _projectId,
472       uint256 _amount,
473       uint256 _currency,
474       address _token,
475       uint256 _minReturnedTokens,
476       address payable _beneficiary,
477       string memory _memo
478     )
479       external
480       virtual
481       override
482       requirePermission(projects.ownerOf(_projectId), _projectId, JBOperations.USE_ALLOWANCE)
483:      returns (uint256 netDistributedAmount)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L385-L399

File: contracts/JBController.sol

/// @audit token
562     function issueTokenFor(
563       uint256 _projectId,
564       string calldata _name,
565       string calldata _symbol
566     )
567       external
568       virtual
569       override
570       requirePermission(projects.ownerOf(_projectId), _projectId, JBOperations.ISSUE)
571:      returns (IJBToken token)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L562-L571

File: contracts/JBFundingCycleStore.sol

/// @audit fundingCycle
86      function get(uint256 _projectId, uint256 _configuration)
87        external
88        view
89        override
90:       returns (JBFundingCycle memory fundingCycle)

/// @audit fundingCycle
194     function currentOf(uint256 _projectId)
195       external
196       view
197       override
198:      returns (JBFundingCycle memory fundingCycle)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L86-L90

#0 - jack-the-pug

2022-08-07T12:27:59Z

Agreed with the severities

Summary

Gas Optimizations

IssueInstances
1Utility function can save a lot of gas1
2Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate5
3Using calldata instead of memory for read-only arguments in external functions saves gas30
4Avoid contract existence checks by using solidity version 0.8.10 or later15
5State variables should be cached in stack variables rather than re-reading them from storage5
6Multiple accesses of a mapping/array should use a local variable cache38
7The result of external function calls should be cached rather than re-calling the function3
8internal functions only called once can be inlined to save gas2
9<array>.length should not be looked up in every loop of a for-loop14
10++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops15
11Optimize names to save gas33
12Using bools for storage incurs overhead3
13>= costs less gas than >2
14++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)15
15Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead2
16Inverting the condition of an if-else-statement wastes gas1
17Functions guaranteed to revert when called by normal users can be marked payable11

Total: 195 instances over 17 issues

Gas Optimizations

1. Utility function can save a lot of gas

By creating a utility function in IJBSingleTokenPaymentTerminal, which returns a tuple of the three values below, every redemption will save the overhead of two extra external calls, which cost 700 gas each, which will add up to significant savings over the life of a project

There is 1 instance of this issue:

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #1

447          address _token = IJBSingleTokenPaymentTerminal(msg.sender).token();
448  
449          // Get a reference to the terminal's decimals.
450          uint256 _decimals = IJBSingleTokenPaymentTerminal(msg.sender).decimals();
451  
452          // Get areference to the terminal's currency.
453:         uint256 _currency = IJBSingleTokenPaymentTerminal(msg.sender).currency();

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L447-L453

2. Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

There are 5 instances of this issue:

File: contracts/JBController.sol

65      mapping(uint256 => int256) internal _processedTokenTrackerOf;
...
82      mapping(uint256 => mapping(uint256 => mapping(IJBPaymentTerminal => mapping(address => uint256))))
83        internal _packedDistributionLimitDataOf;
...
100     mapping(uint256 => mapping(uint256 => mapping(IJBPaymentTerminal => mapping(address => uint256))))
101:      internal _packedOverflowAllowanceDataOf;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L65-L101

File: contracts/JBDirectory.sol

46      mapping(uint256 => IJBPaymentTerminal[]) private _terminalsOf;
47    
48      /**
49        @notice
50        The project's primary terminal for a token.
51    
52        _projectId The ID of the project to get the primary terminal of.
53        _token The token to get the project's primary terminal of.
54      */
55:     mapping(uint256 => mapping(address => IJBPaymentTerminal)) private _primaryTerminalOf;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L46-L55

File: contracts/JBFundingCycleStore.sol

41      mapping(uint256 => mapping(uint256 => uint256)) private _packedUserPropertiesOf;
...
50      mapping(uint256 => mapping(uint256 => uint256)) private _packedIntrinsicPropertiesOf;
...
59      mapping(uint256 => mapping(uint256 => uint256)) private _metadataOf;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L41-L59

File: contracts/JBSplitsStore.sol

44      mapping(uint256 => mapping(uint256 => mapping(uint256 => uint256))) private _splitCountOf;
...
55      mapping(uint256 => mapping(uint256 => mapping(uint256 => mapping(uint256 => uint256))))
56        private _packedSplitParts1Of;
...
70      mapping(uint256 => mapping(uint256 => mapping(uint256 => mapping(uint256 => uint256))))
71:       private _packedSplitParts2Of;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L44-L71

File: contracts/JBTokenStore.sol

84      mapping(uint256 => uint256) public override unclaimedTotalSupplyOf;
...
93      mapping(address => mapping(uint256 => uint256)) public override unclaimedBalanceOf;
...
101:    mapping(uint256 => bool) public override requireClaimFor;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L84-L101

3. Using calldata instead of memory for read-only arguments in external functions saves gas

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved

There are 30 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit _memo
/// @audit _metadata
385     function redeemTokensOf(
386       address _holder,
387       uint256 _projectId,
388       uint256 _tokenCount,
389       address _token,
390       uint256 _minReturnedTokens,
391       address payable _beneficiary,
392       string memory _memo,
393       bytes memory _metadata
394     )
395       external
396       virtual
397       override
398       requirePermission(_holder, _projectId, JBOperations.REDEEM)
399:      returns (uint256 reclaimAmount)

/// @audit _memo
470     function useAllowanceOf(
471       uint256 _projectId,
472       uint256 _amount,
473       uint256 _currency,
474       address _token,
475       uint256 _minReturnedTokens,
476       address payable _beneficiary,
477       string memory _memo
478     )
479       external
480       virtual
481       override
482       requirePermission(projects.ownerOf(_projectId), _projectId, JBOperations.USE_ALLOWANCE)
483:      returns (uint256 netDistributedAmount)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L385-L399

File: contracts/interfaces/IJBController.sol

/// @audit _groupedSplits
/// @audit _fundAccessConstraints
/// @audit _terminals
138     function launchProjectFor(
139       address _owner,
140       JBProjectMetadata calldata _projectMetadata,
141       JBFundingCycleData calldata _data,
142       JBFundingCycleMetadata calldata _metadata,
143       uint256 _mustStartAtOrAfter,
144       JBGroupedSplits[] memory _groupedSplits,
145       JBFundAccessConstraints[] memory _fundAccessConstraints,
146       IJBPaymentTerminal[] memory _terminals,
147       string calldata _memo
148:    ) external returns (uint256 projectId);

/// @audit _groupedSplits
/// @audit _fundAccessConstraints
/// @audit _terminals
150     function launchFundingCyclesFor(
151       uint256 _projectId,
152       JBFundingCycleData calldata _data,
153       JBFundingCycleMetadata calldata _metadata,
154       uint256 _mustStartAtOrAfter,
155       JBGroupedSplits[] memory _groupedSplits,
156       JBFundAccessConstraints[] memory _fundAccessConstraints,
157       IJBPaymentTerminal[] memory _terminals,
158       string calldata _memo
159:    ) external returns (uint256 configuration);

/// @audit _groupedSplits
/// @audit _fundAccessConstraints
161     function reconfigureFundingCyclesOf(
162       uint256 _projectId,
163       JBFundingCycleData calldata _data,
164       JBFundingCycleMetadata calldata _metadata,
165       uint256 _mustStartAtOrAfter,
166       JBGroupedSplits[] memory _groupedSplits,
167       JBFundAccessConstraints[] memory _fundAccessConstraints,
168       string calldata _memo
169:    ) external returns (uint256);

/// @audit _memo
200     function distributeReservedTokensOf(uint256 _projectId, string memory _memo)
201       external
202:      returns (uint256);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBController.sol#L138-L148

File: contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol

/// @audit _defaultMemo
21      function deployProjectPayer(
22        uint256 _defaultProjectId,
23        address payable _defaultBeneficiary,
24        bool _defaultPreferClaimedTokens,
25        string memory _defaultMemo,
26        bytes memory _defaultMetadata,
27        bool _preferAddToBalance,
28        IJBDirectory _directory,
29        address _owner
30:     ) external returns (IJBProjectPayer projectPayer);

/// @audit _defaultMetadata
21      function deployProjectPayer(
22        uint256 _defaultProjectId,
23        address payable _defaultBeneficiary,
24        bool _defaultPreferClaimedTokens,
25        string memory _defaultMemo,
26        bytes memory _defaultMetadata,
27        bool _preferAddToBalance,
28        IJBDirectory _directory,
29        address _owner
30:     ) external returns (IJBProjectPayer projectPayer);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol#L21-L30

File: contracts/interfaces/IJBProjectPayer.sol

/// @audit _memo
/// @audit _metadata
32      function setDefaultValues(
33        uint256 _projectId,
34        address payable _beneficiary,
35        bool _preferClaimedTokens,
36        string memory _memo,
37        bytes memory _metadata,
38:       bool _defaultPreferAddToBalance

/// @audit _memo
/// @audit _metadata
41      function pay(
42        uint256 _projectId,
43        address _token,
44        uint256 _amount,
45        uint256 _decimals,
46        address _beneficiary,
47        uint256 _minReturnedTokens,
48        bool _preferClaimedTokens,
49        string memory _memo,
50:       bytes memory _metadata

/// @audit _memo
/// @audit _metadata
53      function addToBalanceOf(
54        uint256 _projectId,
55        address _token,
56        uint256 _amount,
57        uint256 _decimals,
58        string memory _memo,
59:       bytes memory _metadata

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjectPayer.sol#L32-L38

File: contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol

/// @audit _amount
62      function recordPaymentFrom(
63        address _payer,
64        JBTokenAmount memory _amount,
65        uint256 _projectId,
66        uint256 _baseWeightCurrency,
67        address _beneficiary,
68        string calldata _memo,
69        bytes calldata _metadata
70      )
71        external
72        returns (
73          JBFundingCycle memory fundingCycle,
74          uint256 tokenCount,
75          IJBPayDelegate delegate,
76:         string memory memo

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol#L62-L76

File: contracts/interfaces/IJBSplitsStore.sol

/// @audit _groupedSplits
28      function set(
29        uint256 _projectId,
30        uint256 _domain,
31:       JBGroupedSplits[] memory _groupedSplits

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsStore.sol#L28-L31

File: contracts/JBController.sol

/// @audit _terminals
/// @audit _memo
410     function launchProjectFor(
411       address _owner,
412       JBProjectMetadata calldata _projectMetadata,
413       JBFundingCycleData calldata _data,
414       JBFundingCycleMetadata calldata _metadata,
415       uint256 _mustStartAtOrAfter,
416       JBGroupedSplits[] calldata _groupedSplits,
417       JBFundAccessConstraints[] calldata _fundAccessConstraints,
418       IJBPaymentTerminal[] memory _terminals,
419       string memory _memo
420:    ) external virtual override returns (uint256 projectId) {

/// @audit _fundAccessConstraints
464     function launchFundingCyclesFor(
465       uint256 _projectId,
466       JBFundingCycleData calldata _data,
467       JBFundingCycleMetadata calldata _metadata,
468       uint256 _mustStartAtOrAfter,
469       JBGroupedSplits[] calldata _groupedSplits,
470       JBFundAccessConstraints[] memory _fundAccessConstraints,
471       IJBPaymentTerminal[] memory _terminals,
472       string memory _memo
473     )
474       external
475       virtual
476       override
477       requirePermission(projects.ownerOf(_projectId), _projectId, JBOperations.RECONFIGURE)
478:      returns (uint256 configuration)

/// @audit _terminals
/// @audit _memo
464     function launchFundingCyclesFor(
465       uint256 _projectId,
466       JBFundingCycleData calldata _data,
467       JBFundingCycleMetadata calldata _metadata,
468       uint256 _mustStartAtOrAfter,
469       JBGroupedSplits[] calldata _groupedSplits,
470       JBFundAccessConstraints[] memory _fundAccessConstraints,
471       IJBPaymentTerminal[] memory _terminals,
472       string memory _memo
473     )
474       external
475       virtual
476       override
477       requirePermission(projects.ownerOf(_projectId), _projectId, JBOperations.RECONFIGURE)
478:      returns (uint256 configuration)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L410-L420

File: contracts/JBSingleTokenPaymentTerminalStore.sol

/// @audit _metadata
313     function recordPaymentFrom(
314       address _payer,
315       JBTokenAmount calldata _amount,
316       uint256 _projectId,
317       uint256 _baseWeightCurrency,
318       address _beneficiary,
319       string calldata _memo,
320       bytes memory _metadata
321     )
322       external
323       override
324       nonReentrant
325       returns (
326         JBFundingCycle memory fundingCycle,
327         uint256 tokenCount,
328         IJBPayDelegate delegate,
329:        string memory memo

/// @audit _memo
/// @audit _metadata
414     function recordRedemptionFor(
415       address _holder,
416       uint256 _projectId,
417       uint256 _tokenCount,
418       string memory _memo,
419       bytes memory _metadata
420     )
421       external
422       override
423       nonReentrant
424       returns (
425         JBFundingCycle memory fundingCycle,
426         uint256 reclaimAmount,
427         IJBRedemptionDelegate delegate,
428:        string memory memo

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L313-L329

4. Avoid contract existence checks by using solidity version 0.8.10 or later

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (700 gas), to check for contract existence for external calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value

There are 15 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit mintTokensOf()
1299:         beneficiaryTokenCount = IJBController(directory.controllerOf(_projectId)).mintTokensOf(

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L1299

File: contracts/JBSingleTokenPaymentTerminalStore.sol

/// @audit totalOutstandingTokensOf()
216       uint256 _totalSupply = IJBController(directory.controllerOf(_projectId))
217:        .totalOutstandingTokensOf(_projectId, _fundingCycle.reservedRate());

/// @audit payParams()
359:        (_weight, memo, delegate) = IJBFundingCycleDataSource(fundingCycle.dataSource()).payParams(

/// @audit token()
447:          address _token = IJBSingleTokenPaymentTerminal(msg.sender).token();

/// @audit decimals()
450:          uint256 _decimals = IJBSingleTokenPaymentTerminal(msg.sender).decimals();

/// @audit currency()
453:          uint256 _currency = IJBSingleTokenPaymentTerminal(msg.sender).currency();

/// @audit totalOutstandingTokensOf()
467:          _totalSupply = IJBController(directory.controllerOf(_projectId)).totalOutstandingTokensOf(

/// @audit redeemParams()
506           (reclaimAmount, memo, delegate) = IJBFundingCycleDataSource(fundingCycle.dataSource())
507:            .redeemParams(_data);

/// @audit distributionLimitOf()
560       (uint256 _distributionLimitOf, uint256 _distributionLimitCurrencyOf) = IJBController(
561         directory.controllerOf(_projectId)
562:      ).distributionLimitOf(

/// @audit token()
566:          IJBSingleTokenPaymentTerminal(msg.sender).token()

/// @audit currency()
577:      uint256 _balanceCurrency = IJBSingleTokenPaymentTerminal(msg.sender).currency();

/// @audit overflowAllowanceOf()
636       (uint256 _overflowAllowanceOf, uint256 _overflowAllowanceCurrency) = IJBController(
637         directory.controllerOf(_projectId)
638:      ).overflowAllowanceOf(

/// @audit token()
642:          IJBSingleTokenPaymentTerminal(msg.sender).token()

/// @audit currency()
653:      uint256 _balanceCurrency = IJBSingleTokenPaymentTerminal(msg.sender).currency();

/// @audit distributionLimitOf()
817       (uint256 _distributionLimit, uint256 _distributionLimitCurrency) = IJBController(
818         directory.controllerOf(_projectId)
819:      ).distributionLimitOf(_projectId, _fundingCycle.configuration, _terminal, _terminal.token());

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L216-L217

5. State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

There are 5 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit fee on line 838
880:              _feeAmount(_leftoverDistributionAmount, fee, _feeDiscount);

/// @audit fee on line 1034
1100:                   : _payoutAmount - _feeAmount(_payoutAmount, fee, _feeDiscount);

/// @audit fee on line 1100
1141:               : _payoutAmount - _feeAmount(_payoutAmount, fee, _feeDiscount);

/// @audit fee on line 1195
1199:       _heldFeesOf[_projectId].push(JBFee(_amount, uint32(fee), uint32(_feeDiscount), _beneficiary));

/// @audit fee on line 1199
1201:       emit HoldFee(_projectId, _amount, fee, _feeDiscount, _beneficiary, msg.sender);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L880

6. Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata

There are 38 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

/// @audit _heldFees[_i] on line 597
598:          _heldFees[_i].fee,

/// @audit _heldFees[_i] on line 598
599:          _heldFees[_i].feeDiscount

/// @audit _heldFees[_i] on line 599
603:        _processFee(_amount, _heldFees[_i].beneficiary);

/// @audit _heldFees[_i] on line 603
605:        emit ProcessFee(_projectId, _amount, true, _heldFees[_i].beneficiary, msg.sender);

/// @audit _heldFees[_i] on line 1400
1402:             _heldFees[_i].amount,

/// @audit _heldFees[_i] on line 1402
1403:             _heldFees[_i].fee,

/// @audit _heldFees[_i] on line 1403
1404:             _heldFees[_i].feeDiscount

/// @audit _heldFees[_i] on line 1410
1411:             _heldFees[_i].fee,

/// @audit _heldFees[_i] on line 1411
1412:             _heldFees[_i].feeDiscount,

/// @audit _heldFees[_i] on line 1412
1413:             _heldFees[_i].beneficiary

/// @audit _heldFees[_i] on line 1413
1417:           refundedFees += _feeAmount(leftoverAmount, _heldFees[_i].fee, _heldFees[_i].feeDiscount);

/// @audit _heldFees[_i] on line 1417
1417:           refundedFees += _feeAmount(leftoverAmount, _heldFees[_i].fee, _heldFees[_i].feeDiscount);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L598

File: contracts/JBOperatorStore.sol

/// @audit _operatorData[_i] on line 137
140:        permissionsOf[_operatorData[_i].operator][msg.sender][_operatorData[_i].domain] = _packed;

/// @audit _operatorData[_i] on line 140
140:        permissionsOf[_operatorData[_i].operator][msg.sender][_operatorData[_i].domain] = _packed;

/// @audit _operatorData[_i] on line 140
143:          _operatorData[_i].operator,

/// @audit _operatorData[_i] on line 143
145:          _operatorData[_i].domain,

/// @audit _operatorData[_i] on line 145
146:          _operatorData[_i].permissionIndexes,

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L140

File: contracts/JBSplitsStore.sol

/// @audit _currentSplits[_i] on line 206
214:            _splits[_j].percent == _currentSplits[_i].percent &&

/// @audit _splits[_j] on line 214
215:            _splits[_j].beneficiary == _currentSplits[_i].beneficiary &&

/// @audit _currentSplits[_i] on line 214
215:            _splits[_j].beneficiary == _currentSplits[_i].beneficiary &&

/// @audit _splits[_j] on line 215
216:            _splits[_j].allocator == _currentSplits[_i].allocator &&

/// @audit _currentSplits[_i] on line 215
216:            _splits[_j].allocator == _currentSplits[_i].allocator &&

/// @audit _splits[_j] on line 216
217:            _splits[_j].projectId == _currentSplits[_i].projectId &&

/// @audit _currentSplits[_i] on line 216
217:            _splits[_j].projectId == _currentSplits[_i].projectId &&

/// @audit _splits[_j] on line 217
219:            _splits[_j].lockedUntil >= _currentSplits[_i].lockedUntil

/// @audit _currentSplits[_i] on line 217
219:            _splits[_j].lockedUntil >= _currentSplits[_i].lockedUntil

/// @audit _splits[_i] on line 231
234:        if (_splits[_i].projectId > type(uint56).max) revert INVALID_PROJECT_ID();

/// @audit _splits[_i] on line 234
237:        _percentTotal = _percentTotal + _splits[_i].percent;

/// @audit _splits[_i] on line 237
245:        if (_splits[_i].preferClaimed) _packedSplitParts1 = 1;

/// @audit _splits[_i] on line 245
247:        if (_splits[_i].preferAddToBalance) _packedSplitParts1 |= 1 << 1;

/// @audit _splits[_i] on line 247
249:        _packedSplitParts1 |= _splits[_i].percent << 2;

/// @audit _splits[_i] on line 249
251:        _packedSplitParts1 |= _splits[_i].projectId << 34;

/// @audit _splits[_i] on line 251
253:        _packedSplitParts1 |= uint256(uint160(address(_splits[_i].beneficiary))) << 90;

/// @audit _splits[_i] on line 253
259:        if (_splits[_i].lockedUntil > 0 || _splits[_i].allocator != IJBSplitAllocator(address(0))) {

/// @audit _splits[_i] on line 259
259:        if (_splits[_i].lockedUntil > 0 || _splits[_i].allocator != IJBSplitAllocator(address(0))) {

/// @audit _splits[_i] on line 259
261:          if (_splits[_i].lockedUntil > type(uint48).max) revert INVALID_LOCKED_UNTIL();

/// @audit _splits[_i] on line 261
264:          uint256 _packedSplitParts2 = uint48(_splits[_i].lockedUntil);

/// @audit _splits[_i] on line 264
266:          _packedSplitParts2 |= uint256(uint160(address(_splits[_i].allocator))) << 48;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L214

7. The result of external function calls should be cached rather than re-calling the function

The instances below point to the second+ call of the function within a single function

There are 3 instances of this issue:

File: contracts/JBController.sol   #1

/// @audit _fundingCycle.dataSource()
657:          msg.sender != address(_fundingCycle.dataSource())

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L657

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #2

/// @audit _terminal.currency()
210:        : _overflowDuring(_terminal, _projectId, _fundingCycle, _terminal.currency());

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L210

File: contracts/JBSingleTokenPaymentTerminalStore.sol   #3

/// @audit fundingCycle.useTotalOverflowForRedemptions()
500:            fundingCycle.useTotalOverflowForRedemptions(),

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L500

8. internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

There are 2 instances of this issue:

File: contracts/JBController.sol   #1

900     function _distributeToReservedTokenSplitsOf(
901       uint256 _projectId,
902       uint256 _domain,
903       uint256 _group,
904       uint256 _amount
905:    ) internal returns (uint256 leftoverAmount) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L900-L905

File: contracts/JBSplitsStore.sol   #2

194     function _set(
195       uint256 _projectId,
196       uint256 _domain,
197       uint256 _group,
198:      JBSplit[] memory _splits

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L194-L198

9. <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

  • storage arrays incur a Gwarmaccess (100 gas)
  • memory arrays use MLOAD (3 gas)
  • calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There are 14 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

1008:     for (uint256 _i = 0; _i < _splits.length; ) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L1008

File: contracts/JBController.sol

913:      for (uint256 _i = 0; _i < _splits.length; _i++) {

1014:     for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L913

File: contracts/JBDirectory.sol

139:      for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {

167:      for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)

275:        for (uint256 _i; _i < _terminals.length; _i++)

276:          for (uint256 _j = _i + 1; _j < _terminals.length; _j++)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L139

File: contracts/JBOperatorStore.sol

85:       for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {

135:      for (uint256 _i = 0; _i < _operatorData.length; _i++) {

165:      for (uint256 _i = 0; _i < _indexes.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L85

File: contracts/JBSingleTokenPaymentTerminalStore.sol

862:      for (uint256 _i = 0; _i < _terminals.length; _i++)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L862

File: contracts/JBSplitsStore.sol

204:      for (uint256 _i = 0; _i < _currentSplits.length; _i++) {

211:        for (uint256 _j = 0; _j < _splits.length; _j++) {

229:      for (uint256 _i = 0; _i < _splits.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L204

10. ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

There are 15 instances of this issue:

File: contracts/JBController.sol

913:      for (uint256 _i = 0; _i < _splits.length; _i++) {

1014:     for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L913

File: contracts/JBDirectory.sol

139:      for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {

167:      for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)

275:        for (uint256 _i; _i < _terminals.length; _i++)

276:          for (uint256 _j = _i + 1; _j < _terminals.length; _j++)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L139

File: contracts/JBFundingCycleStore.sol

724:      for (uint256 i = 0; i < _discountMultiple; i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L724

File: contracts/JBOperatorStore.sol

85:       for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {

135:      for (uint256 _i = 0; _i < _operatorData.length; _i++) {

165:      for (uint256 _i = 0; _i < _indexes.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L85

File: contracts/JBSingleTokenPaymentTerminalStore.sol

862:      for (uint256 _i = 0; _i < _terminals.length; _i++)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L862

File: contracts/JBSplitsStore.sol

204:      for (uint256 _i = 0; _i < _currentSplits.length; _i++) {

211:        for (uint256 _j = 0; _j < _splits.length; _j++) {

229:      for (uint256 _i = 0; _i < _splits.length; _i++) {

304:      for (uint256 _i = 0; _i < _splitCount; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L204

11. Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 33 instances of this issue:

File: contracts/interfaces/IJBAllowanceTerminal.sol

/// @audit useAllowanceOf()
4:    interface IJBAllowanceTerminal {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBAllowanceTerminal.sol#L4

File: contracts/interfaces/IJBController.sol

/// @audit projects(), fundingCycleStore(), tokenStore(), splitsStore(), directory(), reservedTokenBalanceOf(), distributionLimitOf(), overflowAllowanceOf(), totalOutstandingTokensOf(), getFundingCycleOf(), latestConfiguredFundingCycleOf(), currentFundingCycleOf(), queuedFundingCycleOf(), launchProjectFor(), launchFundingCyclesFor(), reconfigureFundingCyclesOf(), issueTokenFor(), changeTokenOf(), mintTokensOf(), burnTokensOf(), distributeReservedTokensOf(), migrate()
18:   interface IJBController is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBController.sol#L18

File: contracts/interfaces/IJBControllerUtility.sol

/// @audit directory()
6:    interface IJBControllerUtility {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBControllerUtility.sol#L6

File: contracts/interfaces/IJBDirectory.sol

/// @audit projects(), fundingCycleStore(), controllerOf(), isAllowedToSetFirstController(), terminalsOf(), isTerminalOf(), primaryTerminalOf(), setControllerOf(), setTerminalsOf(), setPrimaryTerminalOf(), setIsAllowedToSetFirstController()
8:    interface IJBDirectory {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBDirectory.sol#L8

File: contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol

/// @audit deployProjectPayer()
7:    interface IJBETHERC20ProjectPayerDeployer {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol#L7

File: contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol

/// @audit deploySplitsPayer()
7:    interface IJBETHERC20SplitsPayerDeployer {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol#L7

File: contracts/interfaces/IJBFeeGauge.sol

/// @audit currentDiscountFor()
4:    interface IJBFeeGauge {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFeeGauge.sol#L4

File: contracts/interfaces/IJBFundingCycleBallot.sol

/// @audit duration(), stateOf()
8:    interface IJBFundingCycleBallot is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleBallot.sol#L8

File: contracts/interfaces/IJBFundingCycleDataSource.sol

/// @audit payParams(), redeemParams()
23:   interface IJBFundingCycleDataSource is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleDataSource.sol#L23

File: contracts/interfaces/IJBFundingCycleStore.sol

/// @audit latestConfigurationOf(), get(), latestConfiguredOf(), queuedOf(), currentOf(), currentBallotStateOf(), configureFor()
8:    interface IJBFundingCycleStore {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleStore.sol#L8

File: contracts/interfaces/IJBMigratable.sol

/// @audit prepForMigrationOf()
4:    interface IJBMigratable {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBMigratable.sol#L4

File: contracts/interfaces/IJBOperatable.sol

/// @audit operatorStore()
6:    interface IJBOperatable {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBOperatable.sol#L6

File: contracts/interfaces/IJBOperatorStore.sol

/// @audit permissionsOf(), hasPermission(), hasPermissions(), setOperator(), setOperators()
6:    interface IJBOperatorStore {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBOperatorStore.sol#L6

File: contracts/interfaces/IJBPayDelegate.sol

/// @audit didPay()
18:   interface IJBPayDelegate is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayDelegate.sol#L18

File: contracts/interfaces/IJBPaymentTerminal.sol

/// @audit acceptsToken(), currencyForToken(), decimalsForToken(), currentEthOverflowOf(), pay(), addToBalanceOf()
6:    interface IJBPaymentTerminal is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPaymentTerminal.sol#L6

File: contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol

/// @audit projects(), splitsStore(), directory(), prices(), store(), baseWeightCurrency(), payoutSplitsGroup(), heldFeesOf(), fee(), feeGauge(), isFeelessAddress(), migrate(), processFees(), setFee(), setFeeGauge(), setFeelessAddress()
20:   interface IJBPayoutRedemptionPaymentTerminal is

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol#L20

File: contracts/interfaces/IJBPayoutTerminal.sol

/// @audit distributePayoutsOf()
4:    interface IJBPayoutTerminal {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutTerminal.sol#L4

File: contracts/interfaces/IJBPriceFeed.sol

/// @audit currentPrice()
4:    interface IJBPriceFeed {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPriceFeed.sol#L4

File: contracts/interfaces/IJBPrices.sol

/// @audit feedFor(), priceFor(), addFeedFor()
6:    interface IJBPrices {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPrices.sol#L6

File: contracts/interfaces/IJBProjectPayer.sol

/// @audit directory(), defaultProjectId(), defaultBeneficiary(), defaultPreferClaimedTokens(), defaultMemo(), defaultMetadata(), defaultPreferAddToBalance(), setDefaultValues(), pay(), addToBalanceOf()
7:    interface IJBProjectPayer is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjectPayer.sol#L7

File: contracts/interfaces/IJBProjects.sol

/// @audit count(), metadataContentOf(), tokenUriResolver(), createFor(), setMetadataOf(), setTokenUriResolver()
8:    interface IJBProjects is IERC721 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjects.sol#L8

File: contracts/interfaces/IJBReconfigurationBufferBallot.sol

/// @audit finalState(), fundingCycleStore(), finalize()
6:    interface IJBReconfigurationBufferBallot is IJBFundingCycleBallot {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBReconfigurationBufferBallot.sol#L6

File: contracts/interfaces/IJBRedemptionDelegate.sol

/// @audit didRedeem()
18:   interface IJBRedemptionDelegate is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBRedemptionDelegate.sol#L18

File: contracts/interfaces/IJBRedemptionTerminal.sol

/// @audit redeemTokensOf()
4:    interface IJBRedemptionTerminal {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBRedemptionTerminal.sol#L4

File: contracts/interfaces/IJBSingleTokenPaymentTerminal.sol

/// @audit token(), currency()
6:    interface IJBSingleTokenPaymentTerminal is IJBPaymentTerminal {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminal.sol#L6

File: contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol

/// @audit fundingCycleStore(), directory(), prices(), usedDistributionLimitOf(), usedOverflowAllowanceOf(), currentOverflowOf(), currentTotalOverflowOf(), currentReclaimableOverflowOf(), currentReclaimableOverflowOf(), recordPaymentFrom(), recordRedemptionFor(), recordDistributionFor(), recordUsedAllowanceOf(), recordAddedBalanceFor(), recordMigration()
13:   interface IJBSingleTokenPaymentTerminalStore {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol#L13

File: contracts/interfaces/IJBSplitAllocator.sol

/// @audit allocate()
21:   interface IJBSplitAllocator is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitAllocator.sol#L21

File: contracts/interfaces/IJBSplitsPayer.sol

/// @audit defaultSplitsProjectId(), defaultSplitsDomain(), defaultSplitsGroup(), splitsStore(), setDefaultSplits()
8:    interface IJBSplitsPayer is IERC165 {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsPayer.sol#L8

File: contracts/interfaces/IJBSplitsStore.sol

/// @audit projects(), directory(), splitsOf(), set()
9:    interface IJBSplitsStore {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsStore.sol#L9

File: contracts/interfaces/IJBTerminalUtility.sol

/// @audit directory()
6:    interface IJBPaymentTerminalUtility {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTerminalUtility.sol#L6

File: contracts/interfaces/IJBToken.sol

/// @audit totalSupply(), mint(), burn(), approve(), transfer(), transferFrom(), transferOwnership()
4:    interface IJBToken {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBToken.sol#L4

File: contracts/interfaces/IJBTokenStore.sol

/// @audit tokenOf(), projectOf(), projects(), unclaimedBalanceOf(), unclaimedTotalSupplyOf(), totalSupplyOf(), requireClaimFor(), issueFor(), changeFor(), burnFrom(), mintFor(), shouldRequireClaimingFor(), claimFor(), transferFrom()
7:    interface IJBTokenStore {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenStore.sol#L7

File: contracts/interfaces/IJBTokenUriResolver.sol

/// @audit getUri()
4:    interface IJBTokenUriResolver {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenUriResolver.sol#L4

12. Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false to true, after having been true in the past

There are 3 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol   #1

181:    mapping(address => bool) public override isFeelessAddress;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L181

File: contracts/JBDirectory.sol   #2

91:     mapping(address => bool) public override isAllowedToSetFirstController;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L91

File: contracts/JBTokenStore.sol   #3

101:    mapping(uint256 => bool) public override requireClaimFor;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L101

13. >= costs less gas than >

The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas

There are 2 instances of this issue:

File: contracts/JBFundingCycleStore.sol   #1

340:        _mustStartAtOrAfter > block.timestamp ? _mustStartAtOrAfter : block.timestamp

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L340

File: contracts/JBFundingCycleStore.sol   #2

427:        _timestampAfterBallot > _mustStartAtOrAfter ? _timestampAfterBallot : _mustStartAtOrAfter,

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L427

14. ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

There are 15 instances of this issue:

File: contracts/JBController.sol

913:      for (uint256 _i = 0; _i < _splits.length; _i++) {

1014:     for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L913

File: contracts/JBDirectory.sol

139:      for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {

167:      for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)

275:        for (uint256 _i; _i < _terminals.length; _i++)

276:          for (uint256 _j = _i + 1; _j < _terminals.length; _j++)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L139

File: contracts/JBFundingCycleStore.sol

724:      for (uint256 i = 0; i < _discountMultiple; i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L724

File: contracts/JBOperatorStore.sol

85:       for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {

135:      for (uint256 _i = 0; _i < _operatorData.length; _i++) {

165:      for (uint256 _i = 0; _i < _indexes.length; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L85

File: contracts/JBSingleTokenPaymentTerminalStore.sol

862:      for (uint256 _i = 0; _i < _terminals.length; _i++)

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L862

File: contracts/JBSplitsStore.sol

204:      for (uint256 _i = 0; _i < _currentSplits.length; _i++) {

211:        for (uint256 _j = 0; _j < _splits.length; _j++) {

229:      for (uint256 _i = 0; _i < _splits.length; _i++) {

304:      for (uint256 _i = 0; _i < _splitCount; _i++) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L204

15. Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

There are 2 instances of this issue:

File: contracts/interfaces/IJBToken.sol   #1

5:      function decimals() external view returns (uint8);

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBToken.sol#L5

File: contracts/JBFundingCycleStore.sol   #2

318:        uint32 _size;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L318

16. Inverting the condition of an if-else-statement wastes gas

Flipping the true and false blocks instead saves 3 gas

There is 1 instance of this issue:

File: contracts/JBFundingCycleStore.sol   #1

631       uint256 _mustStartAtOrAfter = !_allowMidCycle
632         ? block.timestamp + 1
633:        : block.timestamp - _baseFundingCycle.duration + 1;

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L631-L633

17. Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

There are 11 instances of this issue:

File: contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

622:    function setFee(uint256 _fee) external virtual override onlyOwner {

644:    function setFeeGauge(IJBFeeGauge _feeGauge) external virtual override onlyOwner {

661:    function setFeelessAddress(address _address, bool _flag) external virtual override onlyOwner {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L622

File: contracts/JBDirectory.sol

333     function setIsAllowedToSetFirstController(address _address, bool _flag)
334       external
335       override
336:      onlyOwner

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L333-L336

File: contracts/JBFundingCycleStore.sol

299     function configureFor(
300       uint256 _projectId,
301       JBFundingCycleData calldata _data,
302       uint256 _metadata,
303       uint256 _mustStartAtOrAfter
304:    ) external override onlyController(_projectId) returns (JBFundingCycle memory) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L299-L304

File: contracts/JBPrices.sol

109     function addFeedFor(
110       uint256 _currency,
111       uint256 _base,
112       IJBPriceFeed _feed
113:    ) external override onlyOwner {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBPrices.sol#L109-L113

File: contracts/JBProjects.sol

179:    function setTokenUriResolver(IJBTokenUriResolver _newResolver) external override onlyOwner {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBProjects.sol#L179

File: contracts/JBTokenStore.sol

188     function issueFor(
189       uint256 _projectId,
190       string calldata _name,
191       string calldata _symbol
192:    ) external override onlyController(_projectId) returns (IJBToken token) {

236     function changeFor(
237       uint256 _projectId,
238       IJBToken _token,
239       address _newOwner
240:    ) external override onlyController(_projectId) returns (IJBToken oldToken) {

283     function mintFor(
284       address _holder,
285       uint256 _projectId,
286       uint256 _amount,
287       bool _preferClaimedTokens
288:    ) external override onlyController(_projectId) {

320     function burnFrom(
321       address _holder,
322       uint256 _projectId,
323       uint256 _amount,
324       bool _preferClaimedTokens
325:    ) external override onlyController(_projectId) {

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L188-L192

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