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: 4/105
Findings: 8
Award: $5,370.84
π Selected for report: 2
π Solo Findings: 0
π Selected for report: 0xNineDec
Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xdanial, 0xf15ers, Cheeezzyyyy, Chom, Franfran, GalloDaSballo, Green, IllIllI, Meera, Ruhum, bardamu, cccz, codexploder, defsec, hake, hansfriese, horsefacts, hubble, hyh, jonatascm, kebabsec, oyc_109, pashov, rbserver, simon135, tabish, tintin, zzzitron
14.8726 USDC - $14.87
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
If the oracle price feeds are insufficiently validated, there will be pricing errors leading to the miss-pricing of assets
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 IJBPriceFeed
s to fetch their prices:
File: contracts/JBPrices.sol #1 69 if (_feed != IJBPriceFeed(address(0))) return _feed.currentPrice(_decimals);
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();
The code above does not verify that answeredInRound >= roundID
, or that the timestamp is valid either, because the return values are ignored.
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
1929.6275 USDC - $1,929.63
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
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 }
_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;
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 );
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.
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
π Selected for report: horsefacts
Also found by: 0x1f8b, 0x29A, 0x52, 0xf15ers, AlleyCat, Ch_301, Chom, Franfran, IllIllI, Kaiziron, Limbooo, Meera, Ruhum, Sm4rty, apostle0x01, berndartmueller, cccz, cloudjunky, codexploder, cryptphi, delfin454000, durianSausage, fatherOfBlocks, hake, hansfriese, hyh, jonatascm, m_Rassska, oyc_109, peritoflores, rajatbeladiya, rbserver, svskaushik, zzzitron
3.4075 USDC - $3.41
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.
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 }
Code inspection
Add require()
s ensuring that the transfer calls return success
#0 - drgorillamd
2022-07-12T18:52:56Z
Duplicate of #281
1157.7765 USDC - $1,157.78
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.
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 }
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);
which is called by the Controller
:
File: contracts/JBController.sol #3 1011 splitsStore.set(_projectId, _fundingCycle.configuration, _groupedSplits);
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;
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, '');
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
422.0095 USDC - $422.01
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.
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.
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;
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;
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 );
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 }
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 }
The transfer of fees and reserves have the same issue.
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
781.4991 USDC - $781.50
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
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.
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);
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 }
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;
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 {
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
π Selected for report: IllIllI
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xDjango, 0xNazgul, 0xNineDec, 0xdanial, 0xf15ers, Bnke0x0, Ch_301, Chandr, Chom, Funen, GimelSec, Hawkeye, JC, Kaiziron, Lambda, Meera, MiloTruck, Noah3o6, Picodes, ReyAdmirado, Rohan16, Sm4rty, TerrierLover, TomJ, Waze, _Adam, __141345__, asutorufos, aysha, berndartmueller, brgltd, cccz, codexploder, defsec, delfin454000, djxploit, durianSausage, fatherOfBlocks, hake, horsefacts, hubble, jayfromthe13th, joestakey, jonatascm, m_Rassska, oyc_109, pashov, rajatbeladiya, rbserver, robee, sach1r0, sahar, samruna, simon135, svskaushik, zzzitron
873.0371 USDC - $873.04
Issue | Instances | |
---|---|---|
1 | Weight of one being used as zero not documented | 1 |
2 | Calls may run out of gas until arrays are reduced in size | 2 |
3 | Dust amounts not compensated, even if not using price oracle | 1 |
4 | Splits can't be locked once the timestamp passes type(uint48).max | 1 |
5 | Unsafe use of transfer() /transferFrom() with IERC20 | 2 |
Total: 7 instances over 5 issues
Issue | Instances | |
---|---|---|
1 | Confusing variable names | 1 |
2 | Return values of approve() not checked | 1 |
3 | Adding a return statement when the function defines a named return variable, is redundant | 4 |
4 | Non-assembly method available | 1 |
5 | constant s should be defined rather than using magic numbers | 37 |
6 | Use a more recent version of solidity | 1 |
7 | Use a more recent version of solidity | 3 |
8 | Use scientific notation (e.g. 1e18 ) rather than exponentiation (e.g. 10**18 ) | 1 |
9 | Constant redefined elsewhere | 11 |
10 | Inconsistent spacing in comments | 1 |
11 | Lines are too long | 49 |
12 | Typos | 17 |
13 | File is missing NatSpec | 29 |
14 | NatSpec is incomplete | 5 |
15 | Event is missing indexed fields | 34 |
16 | Not using the named return variables anywhere in the function is confusing | 6 |
Total: 201 instances over 16 issues
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);
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;
File: contracts/JBDirectory.sol #2 141: if (_terminal.acceptsToken(_token, _projectId)) return _terminal;
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);
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();
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)
File: contracts/JBERC20PaymentTerminal.sol #2 88: : IERC20(token).transferFrom(_from, _to, _amount);
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;
approve()
not checkedNot 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);
return
statement when the function defines a named return variable, is redundantThere are 4 instances of this issue:
File: contracts/JBFundingCycleStore.sol #1 152: if (_isApproved(_projectId, fundingCycle)) return fundingCycle;
File: contracts/JBFundingCycleStore.sol #2 553: if (_fundingCycle.number == 1) return configuration;
File: contracts/JBFundingCycleStore.sol #3 716: if (_baseFundingCycle.discountRate == 0) return weight;
File: contracts/JBFundingCycleStore.sol #4 835: if (_configuration == 0) return fundingCycle;
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 ?
constant
s should be defined rather than using magic numbersEven 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);
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);
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()
File: contracts/JBETHPaymentTerminal.sol /// @audit 18 42: 18, // 18 decimals.
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));
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();
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);
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)));
File: contracts/JBTokenStore.sol /// @audit 18 249: if (_token != IJBToken(address(0)) && _token.decimals() != 18)
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;
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;
File: contracts/JBDirectory.sol #2 2: pragma solidity 0.8.6;
File: contracts/JBSingleTokenPaymentTerminalStore.sol #3 2: pragma solidity 0.8.6;
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));
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;
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;
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;
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;
File: contracts/JBTokenStore.sol /// @audit seen in contracts/JBSplitsStore.sol 56: IJBProjects public immutable override projects;
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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.
File: contracts/JBFundingCycleStore.sol /// @audit instrinsic 47: _projectId The ID of the project to get instrinsic properties of.
File: contracts/JBProjects.sol /// @audit adherance 84: @param _interfaceId The ID of the interface to check for adherance to.
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.
File: contracts/JBSplitsStore.sol /// @audit extention 218: // Allow lock extention.
There are 29 instances of this issue:
File: contracts/interfaces/IJBAllowanceTerminal.sol
File: contracts/interfaces/IJBController.sol
File: contracts/interfaces/IJBControllerUtility.sol
File: contracts/interfaces/IJBDirectory.sol
File: contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol
File: contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol
File: contracts/interfaces/IJBFeeGauge.sol
File: contracts/interfaces/IJBFundingCycleBallot.sol
File: contracts/interfaces/IJBFundingCycleStore.sol
File: contracts/interfaces/IJBMigratable.sol
File: contracts/interfaces/IJBOperatable.sol
File: contracts/interfaces/IJBOperatorStore.sol
File: contracts/interfaces/IJBPaymentTerminal.sol
File: contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol
File: contracts/interfaces/IJBPayoutTerminal.sol
File: contracts/interfaces/IJBPriceFeed.sol
File: contracts/interfaces/IJBPrices.sol
File: contracts/interfaces/IJBProjectPayer.sol
File: contracts/interfaces/IJBProjects.sol
File: contracts/interfaces/IJBReconfigurationBufferBallot.sol
File: contracts/interfaces/IJBRedemptionTerminal.sol
File: contracts/interfaces/IJBSingleTokenPaymentTerminal.sol
File: contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol
File: contracts/interfaces/IJBSplitsPayer.sol
File: contracts/interfaces/IJBSplitsStore.sol
File: contracts/interfaces/IJBTerminalUtility.sol
File: contracts/interfaces/IJBToken.sol
File: contracts/interfaces/IJBTokenStore.sol
File: contracts/interfaces/IJBTokenUriResolver.sol
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)
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)
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)
indexed
fieldsIndex 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);
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);
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: );
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: );
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: );
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);
File: contracts/interfaces/IJBPrices.sol 7: event AddFeed(uint256 indexed currency, uint256 indexed base, IJBPriceFeed feed);
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: );
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);
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: );
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);
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)
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)
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)
#0 - jack-the-pug
2022-08-07T12:27:59Z
Agreed with the severities
π 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
188.6045 USDC - $188.60
Issue | Instances | |
---|---|---|
1 | Utility function can save a lot of gas | 1 |
2 | Multiple address /ID mappings can be combined into a single mapping of an address /ID to a struct , where appropriate | 5 |
3 | Using calldata instead of memory for read-only arguments in external functions saves gas | 30 |
4 | Avoid contract existence checks by using solidity version 0.8.10 or later | 15 |
5 | State variables should be cached in stack variables rather than re-reading them from storage | 5 |
6 | Multiple accesses of a mapping/array should use a local variable cache | 38 |
7 | The result of external function calls should be cached rather than re-calling the function | 3 |
8 | internal functions only called once can be inlined to save gas | 2 |
9 | <array>.length should not be looked up in every loop of a for -loop | 14 |
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 | 15 |
11 | Optimize names to save gas | 33 |
12 | Using bool s for storage incurs overhead | 3 |
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 |
15 | Usage of uints /ints smaller than 32 bytes (256 bits) incurs overhead | 2 |
16 | Inverting the condition of an if -else -statement wastes gas | 1 |
17 | Functions guaranteed to revert when called by normal users can be marked payable | 11 |
Total: 195 instances over 17 issues
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();
address
/ID mappings can be combined into a single mapping
of an address
/ID to a struct
, where appropriateSaves 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;
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;
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;
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;
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;
calldata
instead of memory
for read-only arguments in external
functions saves gasWhen 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)
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);
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);
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
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
File: contracts/interfaces/IJBSplitsStore.sol /// @audit _groupedSplits 28 function set( 29 uint256 _projectId, 30 uint256 _domain, 31: JBGroupedSplits[] memory _groupedSplits
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)
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
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(
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());
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);
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);
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,
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;
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())
File: contracts/JBSingleTokenPaymentTerminalStore.sol #2 /// @audit _terminal.currency() 210: : _overflowDuring(_terminal, _projectId, _fundingCycle, _terminal.currency());
File: contracts/JBSingleTokenPaymentTerminalStore.sol #3 /// @audit fundingCycle.useTotalOverflowForRedemptions() 500: fundingCycle.useTotalOverflowForRedemptions(),
internal
functions only called once can be inlined to save gasNot 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) {
File: contracts/JBSplitsStore.sol #2 194 function _set( 195 uint256 _projectId, 196 uint256 _domain, 197 uint256 _group, 198: JBSplit[] memory _splits
<array>.length
should not be looked up in every loop of a for
-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD
(3 gas)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; ) {
File: contracts/JBController.sol 913: for (uint256 _i = 0; _i < _splits.length; _i++) { 1014: for (uint256 _i; _i < _fundAccessConstraints.length; _i++) {
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++)
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++) {
File: contracts/JBSingleTokenPaymentTerminalStore.sol 862: for (uint256 _i = 0; _i < _terminals.length; _i++)
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++) {
++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
-loopsThe 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++) {
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++)
File: contracts/JBFundingCycleStore.sol 724: for (uint256 i = 0; i < _discountMultiple; i++) {
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++) {
File: contracts/JBSingleTokenPaymentTerminalStore.sol 862: for (uint256 _i = 0; _i < _terminals.length; _i++)
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++) {
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 {
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 {
File: contracts/interfaces/IJBControllerUtility.sol /// @audit directory() 6: interface IJBControllerUtility {
File: contracts/interfaces/IJBDirectory.sol /// @audit projects(), fundingCycleStore(), controllerOf(), isAllowedToSetFirstController(), terminalsOf(), isTerminalOf(), primaryTerminalOf(), setControllerOf(), setTerminalsOf(), setPrimaryTerminalOf(), setIsAllowedToSetFirstController() 8: interface IJBDirectory {
File: contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol /// @audit deployProjectPayer() 7: interface IJBETHERC20ProjectPayerDeployer {
File: contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol /// @audit deploySplitsPayer() 7: interface IJBETHERC20SplitsPayerDeployer {
File: contracts/interfaces/IJBFeeGauge.sol /// @audit currentDiscountFor() 4: interface IJBFeeGauge {
File: contracts/interfaces/IJBFundingCycleBallot.sol /// @audit duration(), stateOf() 8: interface IJBFundingCycleBallot is IERC165 {
File: contracts/interfaces/IJBFundingCycleDataSource.sol /// @audit payParams(), redeemParams() 23: interface IJBFundingCycleDataSource is IERC165 {
File: contracts/interfaces/IJBFundingCycleStore.sol /// @audit latestConfigurationOf(), get(), latestConfiguredOf(), queuedOf(), currentOf(), currentBallotStateOf(), configureFor() 8: interface IJBFundingCycleStore {
File: contracts/interfaces/IJBMigratable.sol /// @audit prepForMigrationOf() 4: interface IJBMigratable {
File: contracts/interfaces/IJBOperatable.sol /// @audit operatorStore() 6: interface IJBOperatable {
File: contracts/interfaces/IJBOperatorStore.sol /// @audit permissionsOf(), hasPermission(), hasPermissions(), setOperator(), setOperators() 6: interface IJBOperatorStore {
File: contracts/interfaces/IJBPayDelegate.sol /// @audit didPay() 18: interface IJBPayDelegate is IERC165 {
File: contracts/interfaces/IJBPaymentTerminal.sol /// @audit acceptsToken(), currencyForToken(), decimalsForToken(), currentEthOverflowOf(), pay(), addToBalanceOf() 6: interface IJBPaymentTerminal is IERC165 {
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
File: contracts/interfaces/IJBPayoutTerminal.sol /// @audit distributePayoutsOf() 4: interface IJBPayoutTerminal {
File: contracts/interfaces/IJBPriceFeed.sol /// @audit currentPrice() 4: interface IJBPriceFeed {
File: contracts/interfaces/IJBPrices.sol /// @audit feedFor(), priceFor(), addFeedFor() 6: interface IJBPrices {
File: contracts/interfaces/IJBProjectPayer.sol /// @audit directory(), defaultProjectId(), defaultBeneficiary(), defaultPreferClaimedTokens(), defaultMemo(), defaultMetadata(), defaultPreferAddToBalance(), setDefaultValues(), pay(), addToBalanceOf() 7: interface IJBProjectPayer is IERC165 {
File: contracts/interfaces/IJBProjects.sol /// @audit count(), metadataContentOf(), tokenUriResolver(), createFor(), setMetadataOf(), setTokenUriResolver() 8: interface IJBProjects is IERC721 {
File: contracts/interfaces/IJBReconfigurationBufferBallot.sol /// @audit finalState(), fundingCycleStore(), finalize() 6: interface IJBReconfigurationBufferBallot is IJBFundingCycleBallot {
File: contracts/interfaces/IJBRedemptionDelegate.sol /// @audit didRedeem() 18: interface IJBRedemptionDelegate is IERC165 {
File: contracts/interfaces/IJBRedemptionTerminal.sol /// @audit redeemTokensOf() 4: interface IJBRedemptionTerminal {
File: contracts/interfaces/IJBSingleTokenPaymentTerminal.sol /// @audit token(), currency() 6: interface IJBSingleTokenPaymentTerminal is IJBPaymentTerminal {
File: contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol /// @audit fundingCycleStore(), directory(), prices(), usedDistributionLimitOf(), usedOverflowAllowanceOf(), currentOverflowOf(), currentTotalOverflowOf(), currentReclaimableOverflowOf(), currentReclaimableOverflowOf(), recordPaymentFrom(), recordRedemptionFor(), recordDistributionFor(), recordUsedAllowanceOf(), recordAddedBalanceFor(), recordMigration() 13: interface IJBSingleTokenPaymentTerminalStore {
File: contracts/interfaces/IJBSplitAllocator.sol /// @audit allocate() 21: interface IJBSplitAllocator is IERC165 {
File: contracts/interfaces/IJBSplitsPayer.sol /// @audit defaultSplitsProjectId(), defaultSplitsDomain(), defaultSplitsGroup(), splitsStore(), setDefaultSplits() 8: interface IJBSplitsPayer is IERC165 {
File: contracts/interfaces/IJBSplitsStore.sol /// @audit projects(), directory(), splitsOf(), set() 9: interface IJBSplitsStore {
File: contracts/interfaces/IJBTerminalUtility.sol /// @audit directory() 6: interface IJBPaymentTerminalUtility {
File: contracts/interfaces/IJBToken.sol /// @audit totalSupply(), mint(), burn(), approve(), transfer(), transferFrom(), transferOwnership() 4: interface IJBToken {
File: contracts/interfaces/IJBTokenStore.sol /// @audit tokenOf(), projectOf(), projects(), unclaimedBalanceOf(), unclaimedTotalSupplyOf(), totalSupplyOf(), requireClaimFor(), issueFor(), changeFor(), burnFrom(), mintFor(), shouldRequireClaimingFor(), claimFor(), transferFrom() 7: interface IJBTokenStore {
File: contracts/interfaces/IJBTokenUriResolver.sol /// @audit getUri() 4: interface IJBTokenUriResolver {
bool
s 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;
File: contracts/JBDirectory.sol #2 91: mapping(address => bool) public override isAllowedToSetFirstController;
File: contracts/JBTokenStore.sol #3 101: mapping(uint256 => bool) public override requireClaimFor;
>=
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
File: contracts/JBFundingCycleStore.sol #2 427: _timestampAfterBallot > _mustStartAtOrAfter ? _timestampAfterBallot : _mustStartAtOrAfter,
++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++) {
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++)
File: contracts/JBFundingCycleStore.sol 724: for (uint256 i = 0; i < _discountMultiple; i++) {
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++) {
File: contracts/JBSingleTokenPaymentTerminalStore.sol 862: for (uint256 _i = 0; _i < _terminals.length; _i++)
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++) {
uints
/ints
smaller than 32 bytes (256 bits) incurs overheadWhen 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);
File: contracts/JBFundingCycleStore.sol #2 318: uint32 _size;
if
-else
-statement wastes gasFlipping 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;
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 {
File: contracts/JBDirectory.sol 333 function setIsAllowedToSetFirstController(address _address, bool _flag) 334 external 335 override 336: onlyOwner
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) {
File: contracts/JBPrices.sol 109 function addFeedFor( 110 uint256 _currency, 111 uint256 _base, 112 IJBPriceFeed _feed 113: ) external override onlyOwner {
File: contracts/JBProjects.sol 179: function setTokenUriResolver(IJBTokenUriResolver _newResolver) external override onlyOwner {
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) {