Juicebox contest - chaduke's results

Please lock all solidity compiler versions for best optimization

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L312 delete _tokenId as it has never needed

One suggestion is to use custom error Revert to process exception in most write transactions

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L177

Use a locked version of solidity compiler for all contracts

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L177 Suggest to change function setTierDelegate to external if it will not be called by the contract itself

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L44-L53 Consider combing the two mappings into one mapping from address->uint256->struct

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721Delegate.sol#L216-L218 use custom error revert instead of require without an error msg

I suggest to lock all solidity compiler versions for all files and use the most recent version

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/libraries/JBTiered721FundingCycleMetadataResolver.sol all the three functions can be simplified to save gas if these functions are called in other write transactions.

change return (_data & 1) == 1; to return _data & 1;

change return ((_data >> 1) & 1) == 1; to return (_data & 2) == 2;

change return ((_data >> 2) & 1) == 1; to return (_data & 4) == 4;

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.sol#L1123 Consider to combine several of mappings Address - X (.....maxTierIdOf, tierBalanceOf, numberOfReservesMintedFor, numberOfBurnedFor, defaultReservedTokenBeneficiaryOf, firstOwnerOf, baseUriOf, tokenUriResolverOf, contractUriOf, encodedIPFSUriOf) into one mapping Address-> Struct to improve code readability and possibly gas saving due to packing In particular define struct NFTContractInfo { mapping(uint256 => uint256) maxTierIdOf; mapping(uint256 => address) _reservedTokenBeneficiaryOf; mapping(uint256 => JBStored721Tier) _storedTierOf; JBTiered721Flags _flagsOf; mapping(uint256 => uint256) _isTierRemoved; uint256 trackedLastSortTierIdOf; uint256 maxTierIdOf; mapping(address => mapping(uint256 => uint256)) tierBalanceOf; .... } Then define a mapping(address => NFTContractInfo) This is just an example, the authors can define several structs as well, to group related variables in one struct, for example.

It might be helpful to refactor all custom erros into one file and import in each contract.

Maybe it is a good idea to unify the concepts of JB721Tier and JBStored721Tier to avoid the conversion between them back and forth. Much code looks like this:

JB721Tier({
        id: _id,
        contributionFloor: _storedTier.contributionFloor,
        lockedUntil: _storedTier.lockedUntil,
        remainingQuantity: _storedTier.remainingQuantity,
        initialQuantity: _storedTier.initialQuantity,
        votingUnits: _storedTier.votingUnits,
        reservedRate: _storedTier.reservedRate,
        reservedTokenBeneficiary: reservedTokenBeneficiaryOf(_nft, _id),
        encodedIPFSUri: encodedIPFSUriOf[_nft][_id],
        allowManualMint: _storedTier.allowManualMint
      });
which wastes much gas.

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L133 as >= is more expensive than < Change the code to the following:

if (_blockNumber < block.number) return _totalTierCheckpoints[_tier].getAtBlock(_blockNumber);
else revert BLOCK_NOT_YET_MINED();

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L147 change the argument from memory data to calldata to save gas

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L160 No need to cache it into _data, user the _setTierDelegatesData calldata variable directly.

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JB721TieredGovernance.sol#L313 change _tier to a calldata argument to save gas

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721Delegate.sol change codeOrigin, store, fundingCycleStore, prices, pricingCurrency, pricingDecimals, creditsOf to immmutable or possiblly private too since they will set once by the constructor to save gas

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721Delegate.sol#L233 check !=0 is cheaper than checking change 240 to : if (_pricing.tiers.length != 0) _store.recordAddTiers(_pricing.tiers);

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721Delegate.sol#L202 First of all, change memory _mintReservesForTiersData to calldata _mintReservesForTiersData to save gas since the argument will never be updated. Second, no need to cache it to _data in 273, read each field from _mintReservesForTiersData directly to save gas.

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721Delegate.sol#L290 First of all, change memory _mintForTiersData to calldata _mintForTiersData to save gas since the argument will never be updated. Second, no need to cache it to _data in 300, read each field from _mintForTiersData directly to save gas.

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/libraries/JBBitmap.sol#L52 since it is a storage variable, it can save gas by changing self[_depth] |= uint256(1 << (_index % 256)); to self[_depth] = self[_depth] | uint256(1 << (_index % 256));
if the function is called within another write transaction

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/libraries/JBIpfsDecoder.sol#L44 if function _toBase58 will be called in another write transaction, then there would be several gas-saving opportunities:

• _source can be changed to a calldata variable as it has never been updated

• _source.length should be cached in a local variable to avoid repeated call in a for loop

• for (uint256 i = 0; i < _source.length; ++i) should be changed to
      for (uint256; i < sourcelength;  unchecked{++i}) to avoid unnecessary initialization, repleated calling of _source.length (use the cached variable), and the unnecessary check
  

• change for (uint256 j = 0; j < digitlength; ++j) to for (uint256 j; j < digitlength; unchecked{++j}) for similar reason

• Line 59, change digitallength++ to ++digitallength to save gas

similar for loop optimization can be done for lines 68, 76, and 84.

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/libraries/JBIpfsDecoder.sol#L66 _array can be changed to a calldata variable as it is never updated in the function

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/libraries/JBIpfsDecoder.sol#L74 _input can be changed to a calldata variable as it is never updated in the function

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/libraries/JBIpfsDecoder.sol#L83 _indices can be changed to a calldata variable as it is never updated in the function

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.sol#L349 change the forloop to iterate from zero to save the first assignment gas

At the end of B721TieredGovernenance, the _add and _subtract functions are not necessary, simply using SafeMath or compiler version > 8.0 will take care of overflow/underflow issues, and if that is not a concern, simply used unchecked{}.

For function payParams in contract JB721Delegate, the returned value delegateAllocations has no dependency on the input, the function can be simplified to save gas

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/abstract/JB721Delegate.sol#L203 change _name and _symbol to calldata to save gas for the _initialize function