Juicebox contest - 0xSmartContract's results

Non-Critical Issues List

Total 8 issues

Low Risk Issues List

Total 2 issues

Suggestions

Total 1 suggestion

[N-01] Insufficient coverage

Description: The test coverage rate of the project is 30%. Testing all functions is best practice in terms of security criteria.

+-----------------------+-------------------+-------------------+-------------------+------------------+
| File                  | % Lines           | % Statements      | % Branches        | % Funcs          |
+======================================================================================================+
| Total                 | 33.35% (790/2369) | 33.75% (947/2806) | 22.86% (267/1168) | 30.35% (163/537) |
+-----------------------+-------------------+-------------------+-------------------+------------------+

Due to its capacity, test coverage is expected to be 100%.

[N-02] 0 address check

Context: JBTiered721DelegateDeployer.sol#L51-L53 JBTiered721DelegateProjectDeployer.sol#L52-L53 JB721Delegate.sol#L212 JBTiered721Delegate.sol#L223-L224

Description: Also check of the address to protect the code from 0x0 address problem just in case. This is best practice or instead of suggesting that they verify address != 0x0, you could add some good NatSpec comments explaining what is valid and what is invalid and what are the implications of accidentally using an invalid address.

Recommendation: like this; if (routerV2== address(0)) revert ADDRESS_ZERO();

[N-03] For modern and more readable code; update import usages

Context: All contracts

Description: Solidity code is also cleaner in another way that might not be noticeable: the struct Point. We were importing it previously with global import but not using it. The Point struct polluted the source code with an unnecessary object we were not using because we did not need it. This was breaking the rule of modularity and modular programming: only import what you need Specific imports with curly braces allow us to apply this rule better.

Recommendation: import {contract1 , contract2} from "filename.sol";

A good example from the ArtGobblers project;

import {Owned} from "solmate/auth/Owned.sol";
import {ERC721} from "solmate/tokens/ERC721.sol";
import {LibString} from "solmate/utils/LibString.sol";
import {MerkleProofLib} from "solmate/utils/MerkleProofLib.sol";
import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol";
import {ERC1155, ERC1155TokenReceiver} from "solmate/tokens/ERC1155.sol";
import {toWadUnsafe, toDaysWadUnsafe} from "solmate/utils/SignedWadMath.sol";

[N-04] Function writing that does not comply with the Solidity Style Guide

Context: JBTiered721Delegate.sol JBTiered721DelegateStore.sol JB721Delegate.sol

Description: Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.

https://docs.soliditylang.org/en/v0.8.17/style-guide.html

Functions should be grouped according to their visibility and ordered:

• constructor
• receive function (if exists)
• fallback function (if exists)
• external
• public
• internal
• private
• within a grouping, place the view and pure functions last

[N-05] State variables should be marked immutable

Context: JB721Delegate.sol#L48-LL62

Recommendation: The projectId and directory state variables should be changed to immutable, also adhering to the Natspec comments.

[N-06] Omissions in Events

Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters

The events should include the new value and old value where possible: Events with no old value;; JBTiered721Delegate.sol#L374 JBTiered721Delegate.sol#L390 JBTiered721Delegate.sol#L406 JBTiered721Delegate.sol#L422

[N-07] Need Fuzzing test

Context: 22 results - 4 files Project uncheckeds list:

contracts\JB721TieredGovernance.sol:
   164:       unchecked {
  165          ++_i;
contracts\JBTiered721Delegate.sol:
  278:       unchecked {
  279          ++_i;
 305:       unchecked {
  306          ++_i;  339          emit RemoveTier(_tierIdsToRemove[_i], msg.sender);
  340:         unchecked {
  341            ++_i;
  353          emit AddTier(_tierIdsAdded[_i], _tiersToAdd[_i], msg.sender);
  354:         unchecked {
  355            ++_i;
  465:       unchecked {
  466          ++_i;
  508:       unchecked {
  509          ++_i;
  681:       unchecked {
  682          ++_i;
contracts\JBTiered721DelegateStore.sol:
   356:       unchecked {
   357          --_i;
   411:       unchecked {
   412          --_i;
   508:       unchecked {
   509          --_i;
   536:       unchecked {
   537          ++_i;
   568:       unchecked {
   569          ++_i;
   788:       unchecked {
   789          ++_i;
   842:       unchecked {
   843          ++_i;
   877      if (_to != address(0)) {
   878:       unchecked {
   879          // increase the tier balance for the beneficiary
   908:       unchecked {
   909          ++_i;
   985        // Make the token ID.
   986:       unchecked {
   987          // Keep a reference to the token ID.
  1065        // Mint the tokens.
  1066:       unchecked {
  1067          // Keep a reference to the token ID.
  1079:       unchecked {
  1080          ++_i;
  1110:       unchecked {
  1111          ++_i;
contracts\abstract\JB721Delegate.sol:
  281  
  282:       unchecked {
  283          ++_i;

Description: In total 4 contracts, 22 unchecked are used, the functions used are critical. For this reason, there must be fuzzing tests in the tests of the project. Not seen in tests.

Recommendation: Use should fuzzing test like Echidna.

As Alberto Cuesta Canada said: Fuzzing is not easy, the tools are rough, and the math is hard, but it is worth it. Fuzzing gives me a level of confidence in my smart contracts that I didn’t have before. Relying just on unit testing anymore and poking around in a testnet seems reckless now.

https://medium.com/coinmonks/smart-contract-fuzzing-d9b88e0b0a05

[N-08] Use a more recent version of Solidity

Context: All contracts

Description: For security, it is best practice to use the latest Solidity version. For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md

Recommendation: Old version of Solidity is used (^0.8.16), newer version can be used (0.8.17)

[L-01] Use safeTransferOwnership instead of transferOwnership function

Context: JBTiered721Delegate.sol#L251 JBTiered721DelegateDeployer.sol#L100

Description: transferOwnership function is used to change Ownership

Use a 2 structure transferOwnership which is safer. safeTransferOwnership, use it is more secure due to 2-stage ownership transfer.

Recommendation: Use Ownable2Step.sol Ownable2Step.sol

 /**
     * @dev The new owner accepts the ownership transfer.
     */
    function acceptOwnership() external {
        address sender = _msgSender();
        require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
        _transferOwnership(sender);
    }
}

[L-02] Use a more recent version of OpenZeppelin dependencies

Context: All contracts

Description: For security, it is best practice to use the latest OZ version.

package.json#L14

"name": "@openzeppelin/contracts",
"description": "Secure Smart Contract library for Solidity",
"version": "^4.6.0",

For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md

Recommendation: Old version of OZ is used (4.6.0), newer version can be used (4.7.3)

https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v4.7.3

[S-01] Add to blacklist function

Description: Cryptocurrency mixing service, Tornado Cash, has been blacklisted in the OFAC. A lot of blockchain companies, token projects, NFT Projects have blacklisted all Ethereum addresses owned by Tornado Cash listed in the US Treasury Department's sanction against the protocol. https://home.treasury.gov/policy-issues/financial-sanctions/recent-actions/20220808

Some of these Projects; USDC Aave Uniswap Balancer Infura Alchemy Opensea dYdX

Transactions from the project by an account funded by Tonadocash or banned by OFAC can lead to legal problems.Especially American citizens may want to get addresses to the blacklist legally, this is not an obligation

If you think that such legal prohibitions should be made directly by validators, this may not be possible: https://www.paradigm.xyz/2022/09/base-layer-neutrality

The ban on Tornado Cash makes little sense, because in the end, no one can prevent people from using other mixer smart contracts, or forking the existing ones. It neither hinders cybercrime, nor privacy.

Here is the most beautiful and close to the project example; Manifold

Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code

     modifier nonBlacklistRequired(address extension) {
         require(!_blacklistedExtensions.contains(extension), "Extension blacklisted");
         _;
     }

Recommended Mitigation Steps add to Blacklist function and modifier.

Gas Optimizations List

Total 14 issues

Suggestions

Total 2 suggestions

[G-01] Changing state variables to Immutable is gas-optimized

Context:

contracts\abstract\JB721Delegate.sol:
   
   56:   uint256 public override projectId;
   57  

   62:   IJBDirectory public override directory;
   63  

Description: Changing state variables to immutable is ~16k gas-optimized in terms of deployment cost.

Gas Report: Deployment

╭──────────────────────────────────────┬─────────────────╮
│ src/test/test.sol:Contract0 contract ┆                 ┆
╞══════════════════════════════════════╪═════════════════╡
│ Deployment Cost                      ┆ Deployment Size ┆
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤
│ 49587                                ┆ 172             ┆
╰──────────────────────────────────────┴─────────────────╯
╭──────────────────────────────────────┬─────────────────╮
│ src/test/test.sol:Contract1 contract ┆                 ┆
╞══════════════════════════════════════╪═════════════════╡
│ Deployment Cost                      ┆ Deployment Size ┆
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┤
│ 33514                                ┆ 213             ┆
╰──────────────────────────────────────┴─────────────────╯

[G-02] Using storage instead of memory for struct saves gas

Context:

contracts\JB721TieredGovernance.sol:

  156:     JBTiered721SetTierDelegatesData memory _data;

contracts\JBTiered721Delegate.sol:

  272        // Get a reference to the data being iterated on.
  273:       JBTiered721MintReservesForTiersData memory _data = _mintReservesForTiersData[_i];

  299        // Get a reference to the data being iterated on.
  300:       JBTiered721MintForTiersData memory _data = _mintForTiersData[_i];

  348        // Record the added tiers in the store.
  349:       uint256[] memory _tierIdsAdded = store.recordAddTiers(_tiersToAdd);


  437      // Get a reference to the project's current funding cycle.
  438:     JBFundingCycle memory _fundingCycle = fundingCycleStore.currentOf(projectId);
  439  

  447      // Record the minted reserves for the tier.
  448:     uint256[] memory _tokenIds = store.recordMintReservesFor(_tierId, _count);

  557        // Keep a reference to the the specific tier IDs to mint.
  558:       uint16[] memory _tierIdsToMint;

Description: When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read.

[G-03] Functions guaranteed to revert when callled by normal users can be marked payable [24 gas per instance]

Context: 

contracts\JBTiered721Delegate.sol:

  320    */
  321:   function adjustTiers(JB721TierParams[] calldata _tiersToAdd, uint256[] calldata _tierIdsToRemove) external override onlyOwner
  322    {

  366    */
  367:   function setDefaultReservedTokenBeneficiary(address _beneficiary) external override onlyOwner {
  368      // Set the beneficiary.

  382    */
  383:   function setBaseUri(string memory _baseUri) external override onlyOwner {
  384      // Store the new value.

  398    */
  399:   function setContractUri(string calldata _contractUri) external override onlyOwner {
  400      // Store the new value.

  414    */
  415:   function setTokenUriResolver(IJBTokenUriResolver _tokenUriResolver) external override onlyOwner {
  416      // Store the new value.

  476    */
  477:   function mintFor(uint16[] memory _tierIds, address _beneficiary) public override onlyOwner
  478      returns (uint256[] memory tokenIds)

Description: If a function modifier or require such as onlyOwner-admin 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.

Recommendation: Functions guaranteed to revert when called by normal users can be marked payable  (for only onlyowner or admin functions)

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    
    function testGas() public {
        c0.foo();
        c1.foo();
    }
}

contract Contract0 {
    uint256 versionNFTDropCollection;
    
    function foo() external {
        versionNFTDropCollection++;
    }
}

contract Contract1 {
    uint256 versionNFTDropCollection;
    
    function foo() external payable {
        versionNFTDropCollection++;
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬───────┬────────┬───────┬─────────╮
│ src/test/GasTest.t.sol:Contract0 contract ┆                 ┆       ┆        ┆       ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═══════╪════════╪═══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 44293                                     ┆ 252             ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg   ┆ median ┆ max   ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ foo                                       ┆ 22308           ┆ 22308 ┆ 22308  ┆ 22308 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴───────┴────────┴───────┴─────────╯
╭───────────────────────────────────────────┬─────────────────┬───────┬────────┬───────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆       ┆        ┆       ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═══════╪════════╪═══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 41893                                     ┆ 240             ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg   ┆ median ┆ max   ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ foo                                       ┆ 22284           ┆ 22284 ┆ 22284  ┆ 22284 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴───────┴────────┴───────┴─────────╯

[G-04] x -= y (x += y) costs more gas than x = x – y (x = x + y) for state variables [32 gas per instance]

Context:

contracts\JBTiered721DelegateStore.sol:
  353        // Increment the total supply with the amount used already.
  354:       supply += _storedTier.initialQuantity - _storedTier.remainingQuantity;
  355  

  408          // Add the tier's voting units.
  409:         units += _balance * _storedTierOf[_nft][_i].votingUnits;
  410  

  505        // Get a reference to the account's balance in this tier.
  506:       balance += tierBalanceOf[_nft][_owner][_i];
  507  

  533      for (uint256 _i; _i < _numberOfTokenIds; ) {
  534:       weight += _storedTierOf[_nft][tierIdOfToken(_tokenIds[_i])].contributionFloor;
  535  

  562        // Add the tier's contribution floor multiplied by the quantity minted.
  563:       weight +=
  564          (_storedTier.contributionFloor *

  826      // Increment the number of reserved tokens minted.
  827:     numberOfReservesMintedFor[msg.sender][_tierId] += _count;
  828  


contracts\libraries\JBIpfsDecoder.sol:
  51        for (uint256 j = 0; j < digitlength; ++j) {
  52:         carry += uint256(digits[j]) * 256;
  53          digits[j] = uint8(carry % 58);

Description: x -= y costs more gas than x = x – y for state variables.

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTest is DSTest {
  
    Contract0 c0;
    Contract1 c1;
    
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    
    function testGas() public {
        c0.swap(2,3);
        c1.swap1(2,3);
    }
}
contract Contract0 {
    uint256 public amountIn = 10;
    
    
    function swap(uint256 amountInToBin, uint256 fee)  external returns (uint256 ) {
       
       return amountIn -= amountInToBin + fee;
    }
}

contract Contract1 {
    uint256 public amountIn = 10;
    
    function swap1(uint256 amountInToBin, uint256 fee)  external returns (uint256 result1) {
       return (amountIn = amountIn - (amountInToBin + fee));
    }
}

Gas Report:

╭──────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/test.sol:Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞══════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                      ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 83017                                ┆ 341             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                        ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ swap                                 ┆ 5454            ┆ 5454 ┆ 5454   ┆ 5454 ┆ 1       │
╰──────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭──────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/test.sol:Contract1 contract ┆                 ┆      ┆        ┆      ┆         │
╞══════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                      ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 83017                                ┆ 341             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                        ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ swap1                                ┆ 5431            ┆ 5431 ┆ 5431   ┆ 5431 ┆ 1       │
╰──────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯

[G-05] Optimize names to save gas [22 gas per instance]

Context:  All Contracts

Description:  Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions. 

Recommendation:  Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the JBTiered721Delegate.sol contract will be the most used; A lower method ID may be given.

Proof of Consept: https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92

JBTiered721Delegate.sol function names can be named and sorted according to METHOD ID

Sighash   |   Function Signature
========================
59283268  =>  _totalRedemptionWeight()
54c6d1f5  =>  firstOwnerOf(uint256)
70a08231  =>  balanceOf(address)
c87b56dd  =>  tokenURI(uint256)
e8a3d485  =>  contractURI()
01ffc9a7  =>  supportsInterface(bytes4)
982e177c  =>  initialize(uint256,IJBDirectory,string,string,IJBFundingCycleStore,string,)
aa35d9c3  =>  mintReservesFor(JBTiered721MintReservesForTiersData[])
980ed8a8  =>  mintFor(JBTiered721MintForTiersData[])
70d4fddb  =>  adjustTiers(JB721TierParams[],uint256[])
df487e26  =>  setDefaultReservedTokenBeneficiary(address)
a0bcfc7f  =>  setBaseUri(string)
ccb4807b  =>  setContractUri(string)
b6bcc40e  =>  setTokenUriResolver(IJBTokenUriResolver)
aa4fb15b  =>  mintReservesFor(uint256,uint256)
6ac6d941  =>  mintFor(uint16[],address)
8c63c8cd  =>  _processPayment(JBDidPayData)
c8fd1720  =>  _didBurn(uint256[])
b9162928  =>  _mintBestAvailableTier(uint256,address,bool)
515dcef0  =>  _mintAll(uint256,uint16[],address)
a8f32397  =>  _redemptionWeightOf(uint256[])
cad3be83  =>  _beforeTokenTransfer(address,address,uint256)
8f811a1c  =>  _afterTokenTransfer(address,address,uint256)
ce650c23  =>  _afterTokenTransferAccounting(address,address,uint256,JB721Tier)

[G-06] Use assembly to write address storage values [33 gas per instance]

Context:

contracts\JBTiered721Delegate.sol:
  369    */
  370:   function setDefaultReservedTokenBeneficiary(address _beneficiary) external override onlyOwner {
  371      // Set the beneficiary.

  385    */
  386:   function setBaseUri(string memory _baseUri) external override onlyOwner {
  387      // Store the new value.

  401    */
  402:   function setContractUri(string calldata _contractUri) external override onlyOwner {
  403      // Store the new value.

  417    */
  418:   function setTokenUriResolver(IJBTokenUriResolver _tokenUriResolver) external override onlyOwner {
  419      // Store the new value.

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTestFoundry is DSTest {
    Contract1 c1;
    Contract2 c2;
    
    function setUp() public {
        c1 = new Contract1();
        c2 = new Contract2();
    }
    
    function testGas() public {
        c1.setRewardTokenAndAmount(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,356);
        c2.setRewardTokenAndAmount(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,356);
    }
}

contract Contract1  {
    address rewardToken ;
    uint256 reward;

    function setRewardTokenAndAmount(address token_, uint256 reward_) external {
        rewardToken = token_;
        reward = reward_;
    }
}

contract Contract2  {
    address rewardToken ;
    uint256 reward;

    function setRewardTokenAndAmount(address token_, uint256 reward_) external {
        assembly {
            sstore(rewardToken.slot, token_)
            sstore(reward.slot, reward_)           

        }
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬───────┬────────┬───────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆       ┆        ┆       ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═══════╪════════╪═══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 50899                                     ┆ 285             ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg   ┆ median ┆ max   ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ setRewardTokenAndAmount                   ┆ 44490           ┆ 44490 ┆ 44490  ┆ 44490 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴───────┴────────┴───────┴─────────╯
╭───────────────────────────────────────────┬─────────────────┬───────┬────────┬───────┬─────────╮
│ src/test/GasTest.t.sol:Contract2 contract ┆                 ┆       ┆        ┆       ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═══════╪════════╪═══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 38087                                     ┆ 221             ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg   ┆ median ┆ max   ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ setRewardTokenAndAmount                   ┆ 44457           ┆ 44457 ┆ 44457  ┆ 44457 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴───────┴────────┴───────┴─────────╯

[G-07] The solady Library's Ownable contract is significantly gas-optimized, which can be used

Description: The project uses the onlyOwner authorization model with the PendingOwnable.sol contract. I recommend using Solady's highly gas optimized contract.

https://github.com/Vectorized/solady/blob/main/src/auth/OwnableRoles.sol

[G-08] Setting the constructor to payable [13 gas per instance]

Context:

contracts\JBTiered721Delegate.sol:
  184  
  185:   constructor() {
  186      codeOrigin = address(this);

contracts\JBTiered721DelegateDeployer.sol:
  45  
  46:   constructor(
  47      JB721GlobalGovernance _globalGovernance,

contracts\JBTiered721DelegateProjectDeployer.sol:
  46    */
  47:   constructor(
  48      IJBController _controller,

Description: You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.

Recommendation: Set the constructor to payable

Proof Of Concept: https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966/5?u=pcaversaccio

The optimizer was turned on and set to 10000 runs

contract GasTestFoundry is DSTest {
    
    Contract1 c1;
    Contract2 c2;
    
    function setUp() public {
        c1 = new Contract1();
        c2 = new Contract2();
    }
    
    function testGas() public {
        c1.x();
        c2.x();
    }
}

contract Contract1 {
    
    uint256 public dummy;
    constructor() payable {
        dummy = 1;
    }
    
    function x() public {

    }
}

contract Contract2 {
    
    uint256 public dummy;
    constructor() {
        dummy = 1;
    }
    
    function x() public {
    }
}

Gas Report

╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 49563                                     ┆ 159             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract2 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 49587                                     ┆ 172             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

[S-01] Use v4.8.0 OpenZeppelin contracts

Description: The upcoming v4.8.0 version of OpenZeppelin provides many small gas optimizations.

https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v4.8.0-rc.0

v4.8.0-rc.0
⛽ Many small optimizations

[S-02] Missing zero-address check in constructor

Description: Missing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly. It also wast gas as it requires the redeployment of the contract.

contracts\JBTiered721DelegateDeployer.sol:
  45  
  46:   constructor(
  47:     JB721GlobalGovernance _globalGovernance,
  48:     JB721TieredGovernance _tieredGovernance,
  49:     JBTiered721Delegate _noGovernance
  50:   ) {
  51:     globalGovernance = _globalGovernance;
  52:     tieredGovernance = _tieredGovernance;
  53:     noGovernance = _noGovernance;
  54:   }
  55