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
Non-critical 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
constants 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
Low Risk Issues
1. Weight of one being used as zero not documented
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);
2. Calls may run out of gas until arrays are reduced in size
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;
3. Dust amounts not compensated, even if not using price oracle
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);
4. Splits can't be locked once the timestamp passes 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();
5. Unsafe use of 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
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
Not 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
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;
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.
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/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/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/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)
Index 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
Summary
Low Risk Issues
type(uint48).max
transfer()
/transferFrom()
withIERC20
Total: 7 instances over 5 issues
Non-critical Issues
approve()
not checkedreturn
statement when the function defines a named return variable, is redundantconstant
s should be defined rather than using magic numbers1e18
) rather than exponentiation (e.g.10**18
)indexed
fieldsTotal: 201 instances over 16 issues
Low Risk Issues
1. Weight of one being used as zero not documented
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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L467-L471
2. Calls may run out of gas until arrays are reduced in size
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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L357
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L141
3. Dust amounts not compensated, even if not using price oracle
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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L385-L390
4. Splits can't be locked once the timestamp passes
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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L261
5. Unsafe use of
transfer()
/transferFrom()
withIERC20
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()
andtransferFrom()
functions on L1 do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast toIERC20
, their function signatures do not match and therefore the calls made, revert (see this link for a test case). Use OpenZeppelin’sSafeERC20
'ssafeTransfer()
/safeTransferFrom()
insteadThere are 2 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L87
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L88
Non-critical Issues
1. Confusing variable names
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 manyThere is 1 instance of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L332
2. Return values of
approve()
not checkedNot all
IERC20
implementationsrevert()
when there's a failure inapprove()
. The function signature has aboolean
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 anythingThere is 1 instance of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L99
3. Adding a
return
statement when the function defines a named return variable, is redundantThere are 4 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L152
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L553
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L716
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L835
4. Non-assembly method available
assembly{ id := chainid() }
=>uint256 id = block.chainid
,assembly { size := extcodesize() }
=>uint256 size = address().code.length
There is 1 instance of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L320
5.
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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L209
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L166
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L232
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBETHPaymentTerminal.sol#L42
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L354
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L63
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L868
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L251
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L249
6. Use a more recent version of solidity
Use a solidity version of at least 0.8.12 to get
string.concat()
to be used instead ofabi.encodePacked(<str>,<str>)
There is 1 instance of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L2
7. Use a more recent version of solidity
Use a solidity version of at least 0.8.13 to get the ability to use
using for
with a list of free functionsThere are 3 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L2
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L2
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L2
8. Use scientific notation (e.g.
1e18
) rather than exponentiation (e.g.10**18
)There is 1 instance of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L868
9. Constant redefined elsewhere
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 alibrary
. 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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L111
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L65
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L61
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L81
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L56
10. Inconsistent spacing in comments
Some lines use
// x
and some use//x
. The instances below point out the usages that don't follow the majority, within each fileThere is 1 instance of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L912
11. Lines are too long
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:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L26
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L23
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBDirectory.sol#L12
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L18
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBOperatorStore.sol#L34
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L19
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBTokenStore.sol#L30
12. Typos
There are 17 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L247
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitAllocator.sol#L27
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L28
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L47
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBProjects.sol#L84
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSingleTokenPaymentTerminalStore.sol#L384
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBSplitsStore.sol#L218
13. File is missing NatSpec
There are 29 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBAllowanceTerminal.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBController.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBControllerUtility.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBDirectory.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFeeGauge.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleBallot.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleStore.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBMigratable.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBOperatable.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBOperatorStore.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPaymentTerminal.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutTerminal.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPriceFeed.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPrices.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjectPayer.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjects.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBReconfigurationBufferBallot.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBRedemptionTerminal.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminal.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSingleTokenPaymentTerminalStore.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsPayer.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsStore.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTerminalUtility.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBToken.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenStore.sol
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenUriResolver.sol
14. NatSpec is incomplete
There are 5 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L247-L254
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L249-L262
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBProjects.sol#L84-L91
15. Event is missing
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 threeindexed
fields if there are three or more fields, and gas usage is not particularly of concern for the events in questionThere are 34 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBController.sol#L19
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBDirectory.sol#L9
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20ProjectPayerDeployer.sol#L8-L19
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBETHERC20SplitsPayerDeployer.sol#L8-L22
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBFundingCycleStore.sol#L9-L16
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPayoutRedemptionPaymentTerminal.sol#L26-L33
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBPrices.sol#L7
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjectPayer.sol#L8-L16
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBProjects.sol#L9-L14
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBSplitsPayer.sol#L15-L27
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/interfaces/IJBTokenStore.sol#L8-L14
16. Not using the named return variables anywhere in the function is confusing
Consider changing the variable to be an unnamed one
There are 6 instances of this issue:
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L385-L399
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBController.sol#L562-L571
https://github.com/jbx-protocol/juice-contracts-v2-code4rena/tree/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBFundingCycleStore.sol#L86-L90