Juicebox V2 contest - __141345__'s results

Incentive to contribute late, after overflow

If the funding target is not met, the fund is not redeemable. However, after the funding target is hit, new contribution will have an advantage of redeemable, compare to early pay.

Suggestion: Distinguish between redemption before and after funding target.

Constant discount rate for long going funding cycles will damper user's incentive to contribute

For any discount rate > 0, and multiple periodic funding cycle, the token received per ETH/USDC contribution would exponentially going to 0, makes the project less and less attractive to users. And eventually not worth contributing anymore. For example, 0.9^20 = 0.12, 0.95^40 = 0.36, for discount rate of 10%, after 20 cycles, the token is only 12%, for discount rate of 5%, only 36% left. Keep in mind they it is exponentially decrease.

If a project is configured by a contract, not being able to change, the later stages of the project could be hard to develop.

Suggestion: Creating alert for long going projects, or contract owned projects.

Reconfiguration can not be cancelled

There is a delay in the reconfiguration, but after submit, it can not be cancelled. Sometimes project operators may make mistakes, or change their mind after community discussion.

Suggestion: Allowing for cancellation for pending reconfiguration.

redemption rate should be > 0

If the redemption rate is set to 0, it is equivalent to not allow redemption, which can be set elsewhere.

Suggestion: Enforce the redemption rate > 0

FOR-LOOPS GAS-SAVING

There are several for loops can be improved to save gas

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

JBDirectory.sol
139:    for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {
167:    for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)
275,276:
        for (uint256 _i; _i < _terminals.length; _i++)
          for (uint256 _j = _i + 1; _j < _terminals.length; _j++)

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

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++) {

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

JBSplitsStore.sol
165:    for (uint256 _i = 0; _i < _groupedSplitsLength; ) {
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++) {

abstract\JBPayoutRedemptionPaymentTerminal.sol
594:    for (uint256 _i = 0; _i < _heldFeeLength; ) {
1008:    for (uint256 _i = 0; _i < _splits.length; ) {
1396:    for (uint256 _i = 0; _i < _heldFeesLength; ) {

1. LOOP LENGTH COULD BE CACHED

suggestion: cache the for loop length to memory before the loop.

2. ++I COSTS LESS GAS COMPARED TO I++ OR I += 1

suggestion: using ++i instead of i++ to increment the value of an uint variable.

3. NO NEED TO EXPLICITLY INITIALIZE VARIABLES WITH DEFAULT VALUES

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address…). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

suggestion:

for (uint i = 0; i < lenth; ++i)

can be written as

for (uint i ; i < lenth; ++i)

FOR LOOP SUMMARY

The for loops can be written as follows, take one example:

uint length = _splits.length;
for (uint i; i < length;) {
    // ... 
    unchecked { ++i; } 
}

> 0 can be replaced with != 0

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

There are multiple if > 0 statements.

Here is one example:

contracts/JBController.sol:438:
    if (_terminals.length > 0) directory.setTerminalsOf(projectId, _terminals);

Guarantted non underflow can be unchecked

JBTokenStore.sol:

344-364:
    if (_claimedBalance == 0)
      _claimedTokensToBurn = 0;
      // If prefer converted, redeem tokens before redeeming unclaimed tokens.
    else if (_preferClaimedTokens)
      _claimedTokensToBurn = _claimedBalance < _amount ? _claimedBalance : _amount;
      // Otherwise, redeem unclaimed tokens before claimed tokens.
    else _claimedTokensToBurn = _unclaimedBalance < _amount ? _amount - _unclaimedBalance : 0;

    // The amount of unclaimed tokens to redeem.
    uint256 _unclaimedTokensToBurn = _amount - _claimedTokensToBurn;

    // Subtract the tokens from the unclaimed balance and total supply.
    if (_unclaimedTokensToBurn > 0) {
      // Reduce the holders balance and the total supply.
      unclaimedBalanceOf[_holder][_projectId] =
        unclaimedBalanceOf[_holder][_projectId] -
        _unclaimedTokensToBurn;
      unclaimedTotalSupplyOf[_projectId] =
        unclaimedTotalSupplyOf[_projectId] -
        _unclaimedTokensToBurn;
    }

406-412:
    if (_unclaimedBalance < _amount) revert INSUFFICIENT_UNCLAIMED_TOKENS();

    // Subtract the claim amount from the holder's unclaimed project token balance.
    unclaimedBalanceOf[_holder][_projectId] = unclaimedBalanceOf[_holder][_projectId] - _amount;

    // Subtract the claim amount from the project's unclaimed total supply.
    unclaimedTotalSupplyOf[_projectId] = unclaimedTotalSupplyOf[_projectId] - _amount;

445-448:
    if (_amount > _unclaimedBalance) revert INSUFFICIENT_UNCLAIMED_TOKENS();

    // Subtract from the holder's unclaimed token balance.
    unclaimedBalanceOf[_holder][_projectId] = unclaimedBalanceOf[_holder][_projectId] - _amount;


JBFundingCycleStore.sol
561:  block.timestamp < _fundingCycle.start - _baseFundingCycle.duration
589:  _fundingCycle.duration > 0 && block.timestamp >= _fundingCycle.start + _fundingCycle.duration
602:  block.timestamp >= _baseFundingCycle.start + _baseFundingCycle.duration
673:  uint256 _nextImmediateStart = _baseFundingCycle.start + _baseFundingCycle.duration;

679-686:
    uint256 _timeFromImmediateStartMultiple = (_mustStartAtOrAfter - _nextImmediateStart) %
      _baseFundingCycle.duration;

    // A reference to the first possible start timestamp.
    start = _mustStartAtOrAfter - _timeFromImmediateStartMultiple;

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

X = X + Y IS CHEAPER THAN X += Y

Consider use X = X + Y to save gas

contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol

860:    _feeEligibleDistributionAmount += _leftoverDistributionAmount;
1038:   feeEligibleDistributionAmount += _payoutAmount;
1103:   feeEligibleDistributionAmount += _payoutAmount;
1145:   feeEligibleDistributionAmount += _payoutAmount;
1401:   refundedFees += _feeAmount(
1417:   refundedFees += _feeAmount(leftoverAmount, _heldFees[_i].fee, _heldFees[_i].feeDiscount);

pragma solidity 0.8.6;

contract TestPE {
    function test(uint256 a, uint256 b) external returns(uint256) { // 25644
        a += b;
        return a;
    }
}

contract TestEP {
    function test(uint256 a, uint256 b) external returns(uint256) { // 25644
        a = a + b;
        return a;
    }
}