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
Rank: 76/105
Findings: 1
Award: $45.80
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xA5DF
Also found by: 0v3rf10w, 0x09GTO, 0x1f8b, 0x29A, 0xDjango, 0xKitsune, 0xNazgul, 0xdanial, 0xf15ers, Aymen0909, Bnke0x0, Ch_301, Cheeezzyyyy, Chom, ElKu, Funen, Hawkeye, IllIllI, JC, JohnSmith, Kaiziron, Lambda, Limbooo, Meera, Metatron, MiloTruck, Noah3o6, Picodes, Randyyy, RedOneN, ReyAdmirado, Rohan16, Saintcode_, Sm4rty, TomJ, Tomio, Tutturu, UnusualTurtle, Waze, _Adam, __141345__, ajtra, apostle0x01, asutorufos, brgltd, c3phas, cRat1st0s, codexploder, defsec, delfin454000, djxploit, durianSausage, exd0tpy, fatherOfBlocks, hake, horsefacts, ignacio, jayfromthe13th, joestakey, jonatascm, kaden, kebabsec, m_Rassska, mektigboy, mrpathfindr, oyc_109, rajatbeladiya, rbserver, rfa, robee, sach1r0, sashik_eth, simon135
45.8024 USDC - $45.80
If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). If you explicitly initialize it with its default value, you are just wasting deployment gas. It costs more deployment gas to initialize state variables to zero than to let the default of zero be applied.
File: JBProjects.sol line 40
uint256 public override count = 0;
Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block
File: JBTokenStore.sol line 409
unclaimedBalanceOf[_holder][_projectId] = unclaimedBalanceOf[_holder][_projectId] - _amount;
The above line cannot underflow due to the check on line 406 which ensures that the above arithmetic operation can only be performed if unclaimedBalanceOf[_holder][_projectId]
is greater than _amount
We can therefore modify the code to:
unchecked{ unclaimedBalanceOf[_holder][_projectId] = unclaimedBalanceOf[_holder][_projectId] - _amount; }
File: JBSingleTokenPaymentTerminalStore.sol line 834
return _balanceOf > _distributionLimitRemaining ? _balanceOf - _distributionLimitRemaining : 0;
The arithmetic operation _balanceOf - _distributionLimitRemaining
can not underflow since this operation would only be performed if _balanceOf
is greater than _distributionLimitRemaining
File: JBTokenStore.sol line 350
else _claimedTokensToBurn = _unclaimedBalance < _amount ? _amount - _unclaimedBalance : 0;
The arithmetic operation _amount - _unclaimedBalance
cannot underflow since this operation would only be performed if _amount
is greater than _unclaimedBalance
File: JBTokenStore.sol line 448
unclaimedBalanceOf[_holder][_projectId] = unclaimedBalanceOf[_holder][_projectId] - _amount;
The above line cannot underflow due the check on line 445 which ensures that the arithmetic operation would only be perfomed if unclaimedBalanceOf[_holder][_projectId]
is greater than _amount
File: JBSingleTokenPaymentTerminalStore.sol line 519
balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] = balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] - reclaimAmount;
The above cannot underflow due to the check on line 514 which ensures that the above operation is only performed if balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId]
is greater than reclaimAmount
File: JBSingleTokenPaymentTerminalStore.sol line 598
balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] = balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId] - distributedAmount;
The above cannot underflow due to the check on line 589 which ensures that balanceOf[IJBSingleTokenPaymentTerminal(msg.sender)][_projectId]
is greater than distributedAmount
before performing our subtraction arithmetic
File: JBTokenStore.sol line 353
uint256 _unclaimedTokensToBurn = _amount - _claimedTokensToBurn;
The above line cannot underflow due to the checks on line 344, line 348 and line 350 which ensures that the value of _claimedTokensToBurn
is less than amount
before the arithmetic operation is done.
The majority of Solidity for loops increment a uint256 variable that starts at 0. These increment operations never need to be checked for over/underflow because the variable will never reach the max number of uint256 (will run out of gas long before that happens). The default over/underflow check wastes gas in every iteration of virtually every for loop . eg.
e.g Let's work with a sample loop below.
for(uint256 i; i < 10; i++){ //doSomething }
can be written as shown below.
for(uint256 i; i < 10;) { // loop logic unchecked { i++; } }
We can also write it as an inlined function like below.
function inc(i) internal pure returns (uint256) { unchecked { return i + 1; } } for(uint256 i; i < 10; i = inc(i)) { // doSomething }
Affected code
File: JBController.sol line 913
for (uint256 _i = 0; _i < _splits.length; _i++) {
The above should be modified to:
for (uint256 _i = 0; _i < _splits.length;) { unchecked{ ++i; }
Other Instances to modify File: JBController.sol line 1014
for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {
File: JBDirectory.sol line 139
for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {
File: JBDirectory.sol line 167
for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)
File: JBDirectory.sol line 275
for (uint256 _i; _i < _terminals.length; _i++)
File: JBFundingCycleStore.sol line 724
for (uint256 i = 0; i < _discountMultiple; i++) {
File: JBOperatorStore.sol line 85
for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {
File: JBOperatorStore.sol line 135
for (uint256 _i = 0; _i < _operatorData.length; _i++) {
File: JBOperatorStore.sol line 165
for (uint256 _i = 0; _i < _indexes.length; _i++) {
File: JBSingleTokenPaymentTerminalStore.sol line 862
for (uint256 _i = 0; _i < _terminals.length; _i++)
File: JBSplitsStore.sol line 204
for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
File:JBSplitsStore.sol line 211
for (uint256 _j = 0; _j < _splits.length; _j++) {
File:JBSplitsStore.sol line 229
for (uint256 _i = 0; _i < _splits.length; _i++) {
File:JBSplitsStore.sol line 304
for (uint256 _i = 0; _i < _splitCount; _i++) {
See
File: JBSplitsStore.sol line 165
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
The solidity compiler will always read the length of the array during each iteration. That is,
1.if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929 2) for each iteration except for the first), 2.if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first), 3.if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)
This extra costs can be avoided by caching the array length (in stack): When reading the length of an array, sload or mload or calldataload operation is only called once and subsequently replaced by a cheap dupN instruction. Even though mload , calldataload and dupN have the same gas cost, mload and calldataload needs an additional dupN to put the offset in the stack, i.e., an extra 3 gas. which brings this to 6 gas
Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead:
File: JBController.sol line 913
for (uint256 _i = 0; _i < _splits.length; _i++) {
The above should be modified to
uint256 length = _splits.length; for (uint256 _i = 0; _i < length; _i++) {
Other instances to modify File: JBController.sol line 1014
for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {
File: JBDirectory.sol line 275
for (uint256 _i; _i < _terminals.length; _i++)
File: JBOperatorStore.sol line 85
for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {
File: JBOperatorStore.sol line 135
for (uint256 _i = 0; _i < _operatorData.length; _i++) {
File: JBOperatorStore.sol line 165
for (uint256 _i = 0; _i < _indexes.length; _i++) {
File: JBSingleTokenPaymentTerminalStore.sol line 862
for (uint256 _i = 0; _i < _terminals.length; _i++)
File: JBSplitsStore.sol line 204
for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
File:JBSplitsStore.sol line 211
for (uint256 _j = 0; _j < _splits.length; _j++) {
File:JBSplitsStore.sol line 229
for (uint256 _i = 0; _i < _splits.length; _i++) {
File:JBPayoutRedemptionPaymentTerminal.sol line 1008
for (uint256 _i = 0; _i < _splits.length; ) {
Something similar to my proposal has already been implemented on some contracts. See File: JBSplitsStore.sol line 165
++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.
i++ increments i and returns the initial value of i. Which means:
uint i = 1; i++; // == 1 but i == 2
But ++i returns the actual incremented value:
uint i = 1; ++i; // == 2 and i == 2 too, so no need for a temporary variable
In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2
Instances include: File: JBController.sol line 913
for (uint256 _i = 0; _i < _splits.length; _i++) {
File: JBController.sol line 1014
for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {
File: JBDirectory.sol line 139
for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++) {
File: JBDirectory.sol line 167
for (uint256 _i; _i < _terminalsOf[_projectId].length; _i++)
File: JBDirectory.sol line 275
for (uint256 _i; _i < _terminals.length; _i++)
File: JBFundingCycleStore.sol line 724
for (uint256 i = 0; i < _discountMultiple; i++) {
File: JBOperatorStore.sol line 85
for (uint256 _i = 0; _i < _permissionIndexes.length; _i++) {
File: JBOperatorStore.sol line 135
for (uint256 _i = 0; _i < _operatorData.length; _i++) {
File: JBOperatorStore.sol line 165
for (uint256 _i = 0; _i < _indexes.length; _i++) {
File: JBSingleTokenPaymentTerminalStore.sol line 862
for (uint256 _i = 0; _i < _terminals.length; _i++)
File: JBSplitsStore.sol line 204
for (uint256 _i = 0; _i < _currentSplits.length; _i++) {
File:JBSplitsStore.sol line 211
for (uint256 _j = 0; _j < _splits.length; _j++) {
File:JBSplitsStore.sol line 229
for (uint256 _i = 0; _i < _splits.length; _i++) {
File:JBSplitsStore.sol line 304
for (uint256 _i = 0; _i < _splitCount; _i++) {