Juicebox contest - ReyAdmirado's results

1. typo in comments

superceeds --> supersedes

• JBTiered721DelegateStore.sol#L176

referecen --> reference

• JBTiered721DelegateStore.sol#L230

Initialise --> Initialize (this is repeated in many places, because there is function Initialize being used in other places better to keep it american and use this version)

accross --> across

• JBTiered721DelegateStore.sol#L497

• JBTiered721Delegate.sol#L121

benficiary --> beneficiary

• JBTiered721DelegateStore.sol#L597

• JBTiered721Delegate.sol#L363

• JBTiered721Delegate.sol#L368

reservd --> reserved

• JBTiered721DelegateStore.sol#L852

transfered --> transferred

• JBTiered721DelegateStore.sol#L862

• JBTiered721Delegate.sol#L721

• JBTiered721Delegate.sol#L728

• JBTiered721Delegate.sol#L757

• JBTiered721Delegate.sol#L782

• JB721GlobalGovernance.sol#L48

preconfifigured --> preconfigured

• JBTiered721DelegateProjectDeployer.sol#L19

adherance --> adherence

• JBTiered721Delegate.sol#L173

• JB721Delegate.sol#L176

provded --> provided

• JBTiered721Delegate.sol#L545

regitered --> registered

• JBTiered721Delegate.sol#L717

2. use of floating pragma

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

• JBTiered721Delegate.sol
• JBTiered721DelegateDeployer.sol
• JBTiered721DelegateProjectDeployer.sol
• JBTiered721DelegateStore.sol

3. require()/revert() statements should have descriptive reason strings or custom error

• JBTiered721Delegate.sol#L216
• JBTiered721Delegate.sol#L218

4. _safemint() should be used rather than _mint() wherever possible

• JBTiered721Delegate.sol#L461
• JBTiered721Delegate.sol#L504
• JBTiered721Delegate.sol#L635
• JBTiered721Delegate.sol#L677

5. lines are too long

Usually lines in source code are limited to 80 characters. Its advised to keep lines lower than 120 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

• JBTiered721DelegateProjectDeployer.sol#L15

• JBTiered721DelegateProjectDeployer.sol#L19

• JBTiered721DelegateDeployer.sol#L15

• JBTiered721Delegate.sol#L17

• JBTiered721Delegate.sol#L198

• JBTiered721Delegate.sol#L545

1. State variables only set in the constructor should be declared immutable.

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmaccess (100 gas) with a PUSH32 (3 gas).

• JBTiered721Delegate.sol#L48

2. <x> += <y> costs more gas than <x> = <x> + <y> for state variables

Using the addition operator instead of plus-equals saves gas

• JBTiered721DelegateStore.sol#L827

3. using calldata instead of memory for read-only arguments in external functions saves gas

When 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.

• JBTiered721DelegateStore.sol#L628

• JBTiered721DelegateStore.sol#L291

• JBTiered721DelegateProjectDeployer.sol#L70

• JBTiered721DelegateProjectDeployer.sol#L107

• JBTiered721DelegateProjectDeployer.sol#L150

• JBTiered721DelegateDeployer.sol#L71

• JBTiered721Delegate.sol#L264

• JBTiered721Delegate.sol#L290

• JBTiered721Delegate.sol#L386

4. using bool 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. 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

• JBTiered721DelegateStore.sol#L1015

• JBTiered721Delegate.sol#L543

• JBTiered721Delegate.sol#L546

• JBTiered721Delegate.sol#L555

• JBTiered721Delegate.sol#L616

5. internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

• JBTiered721DelegateProjectDeployer.sol#L191

• JBTiered721DelegateProjectDeployer.sol#L216

• JBTiered721DelegateProjectDeployer.sol#L242

• JBTiered721Delegate.sol#L524

• JBTiered721Delegate.sol#L598

• JBTiered721Delegate.sol#L613

• JBTiered721Delegate.sol#L650

• JBTiered721Delegate.sol#L695

• JBTiered721Delegate.sol#L711

• JBTiered721Delegate.sol#L785

6. bytes constants are more efficient than string constants

If data can fit into 32 bytes, then you should use bytes32 datatype rather than bytes or strings as it is cheaper in solidity.

• JBTiered721DelegateStore.sol#L172

• JBTiered721DelegateStore.sol#L188

• JBTiered721DelegateStore.sol#L1133

• JBTiered721DelegateStore.sol#L1143

• JBTiered721Delegate.sol#L205

• JBTiered721Delegate.sol#L206

• JBTiered721Delegate.sol#L208

• JBTiered721Delegate.sol#L210

• JBTiered721Delegate.sol#L386

• JBTiered721Delegate.sol#L402

7. public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents’ functions and change the visibility from external to public and can save gas by doing so.

• JBTiered721DelegateStore.sol#L550