Fractional v2 contest - sach1r0's results

Lack of zero-address check in constructor

Details

It's important to check for zero-address to avoid redeploying of the contract when the address is accidentally set to zero-address.

Mitigation

Add a require statement, for example: require(_registry != address(0));

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L42-L50 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Migration.sol#L53-L60 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Minter.sol#L17-L19 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L24-L26 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/references/SupplyReference.sol#L15-L17 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/targets/Supply.sol#L16-L18

---

Open TODO

Open TODO is present in src/utils/MerkleBase.sol.

Mitigation

I suggest avoiding open TODOs as they may indicate errors that still needs to be fixed

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/utils/MerkleBase.sol#L24-L25

---

Typo

Details

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/interfaces/IMigration.sol#L23 succesful -> successful

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/interfaces/IMigration.sol#L29 succesfully -> successfully

Functions that are not called within the contract must set its visibility to external instead of public

Details

Setting function's visibility to external when it is only called externally can save gas because external function’s parameters are not copied into memory and are instead read from calldata directly. see reference: https://github.com/code-423n4/2021-06-gro-findings/issues/37

Mitigation

Set function visibility to external

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/FERC1155.sol#L256-L287

---

No need to explicitly initialize variables with their default values

Details

When variables are not set, it is assumed to have it's default value(0 for uint, false for bool, address(0) for address). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

see reference: https://code4rena.com/reports/2022-02-jpyc/ [G-07] GENERAL RECOMMENDATIONS

Mitigation

change uint256 i = 0; to uint256 i;

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L78 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L104 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L64 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L83 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L107 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/utils/MerkleBase.sol#L51

---

Pre-increment cost less gas than post-increment

Details

i++ costs more gas than ++i , for uint pre-decrement is cheaper than post-decrement see reference: https://github.com/code-423n4/2021-12-nftx-findings/issues/195

Mitigation

change i++ to ++i

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L78 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L104

---

Variables: "constants" expressions are expressions and not constants

Details

Due to how constant variables are implemented, an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas. Using immutable however, will save gas as the calculation would only be done once during deploy time and the result would be saved and read directly at runtime. see reference: https://github.com/code-423n4/2022-03-lifinance-findings/issues/44#constants-expressions-are-expressions-not-constants

Mitigation

Change these expressions from constant to immutable and implement the calculation in the constructor or hardcode these values in the constants and add a comment to say how the value was calculated.

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/constants/Permit.sol#L5-L7 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/constants/Permit.sol#L10-L12 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/constants/Permit.sol#L15-L17

---

Solidity compiler will always read the length of the array during each iteration

Details

.length in a loop can be extracted into a variable and used where necessary to reduce the number of storage reads see reference: https://github.com/code-423n4/2021-10-union-findings/issues/92

Mitigation:

This extra costs can be avoided by caching the array length. Example: uint _tokensLength = _tokens.length; for (uint i = 0; i < tokensLength; ) { }

Line of code

https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L64 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L83 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L107 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L130 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/protoforms/BaseVault.sol#L132 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/utils/MerkleBase.sol#L51 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/utils/MerkleBase.sol#L110