Notional x Index Coop - catchup's results

A collaboration between Notional and Index Coop to create fixed rate yield index tokens.

General Information

Platform: Code4rena

Start Date: 07/06/2022

Pot Size: $75,000 USDC

Total HM: 11

Participants: 77

Period: 7 days

Judge: gzeon

Total Solo HM: 7

Id: 124

League: ETH

Notional

Findings Distribution

Researcher Performance

Rank: 39/77

Findings: 2

Award: $135.76

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Notional

Findings Information

🌟 Selected for report: berndartmueller

Also found by: 0x1f8b, 0x29A, 0xDjango, 0xNazgul, 0xNineDec, 0xf15ers, 0xkatana, 0xmint, Bronicle, Chom, Cityscape, Deivitto, Funen, GimelSec, GreyArt, IllIllI, JC, Lambda, Meera, Nethermind, Picodes, PierrickGT, Ruhum, Sm4rty, Tadashi, TerrierLover, TomJ, Trumpero, Waze, _Adam, antonttc, ayeslick, c3phas, catchup, cccz, cloudjunky, cryptphi, csanuragjain, delfin454000, dipp, ellahi, fatherOfBlocks, hake, hansfriese, hyh, joestakey, jonah1005, kenzo, minhquanym, oyc_109, sach1r0, saian, simon135, slywaters, sorrynotsorry, sseefried, unforgiven, xiaoming90, z3s, zzzitron

Awards

88.1603 USDC - $88.16

Labels

bug

QA (Quality Assurance)

External Links

Link to GitHub issue

Floating pragma and different compiler versions among contracts

Some of the contracts are using solidity version 0.6.10, whereas others use 0.8.11. Also wfCashERC4626.sol is using floating pragma ^0.8.0. 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. It is recommended to use the same solidity version for all the contracts. I also suggest to use a newer solidity version rather than 0.6.10 to benefit new features and bugfixes.

References: https://swcregistry.io/docs/SWC-103 https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used

Missing non-zero amount checks

It is a good practice to apply non-zero amount checks for token transactions to avoid unnecessary executions.

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashLogic.sol#L158 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashLogic.sol#L166 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashLogic.sol#L184

Events missing indexed fields

Each event can have up to 3 indexed fields. Some events don't have as much indexed fields as they can.

Lines of code

Missing natspec comments

Some functions do not have any natspec comments and most of them are missing @param and @return arguments.

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashBase.sol#L142 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L21 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L26 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L39

Typo in the comment

"does not require and additional redeem call to asset tokens" -> I think "and" should be "an"

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashLogic.sol#L66

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xSolus, 0xf15ers, 0xkatana, 0xmint, 8olidity, Chom, Cityscape, DavidGialdi, Deivitto, ElKu, Fitraldys, Funen, GreyArt, Lambda, Meera, Picodes, PierrickGT, Sm4rty, Tadashi, TerrierLover, TomJ, Tomio, UnusualTurtle, Waze, _Adam, antonttc, asutorufos, berndartmueller, c3phas, catchup, csanuragjain, delfin454000, djxploit, ellahi, fatherOfBlocks, hake, hansfriese, hyh, joestakey, kaden, minhquanym, oyc_109, rfa, sach1r0, saian, samruna, simon135, slywaters, ynnad

Awards

47.6016 USDC - $47.60

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

caching components.length would save SLOAD's

There are for loops which use components.length as the loop boundary. components.length can be cached and used from stack rather than reading from state for each iteration.

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/SetToken.sol#L485 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/SetToken.sol#L604 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/SetToken.sol#L636

Redundant initialisation with default value

Some variables are initialised with their default values which cause unnecessary gas consumption

Lines of code

Prefix increment/decrements are cheaper than postfix increment/decrements

Lines of code

Using != 0 is cheaper than > 0 when used on a uint in a require() statement with the optimizer enabled

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol#L119 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol#L174 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashBase.sol#L37 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L23

Error strings longer than 32 characters

Error strings take space in the deployed bytecode. Every reason string takes at least 32 bytes so make sure your string fits in 32 bytes or it will become more expensive.

Lines of code

There are many lines, some of which are: https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol#L220 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol#L317 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol#L318 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/DebtIssuanceModule.sol#L336

It is better to use 1-2 for binary variables rather than 0-1

SSTORE from 1 to 2 is cheaper than SSTORE from 0 to 1. Therefore, some booleans such as isLocked, anySetAllowed, etc can be defined as uint and their values can be switched between 1-2. https://github.com/code-423n4/2022-01-yield-findings/issues/102

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L118 https://github.com/code-423n4/2022-06-notional-coop/blob/main/index-coop-notional-trade-module/contracts/protocol/SetToken.sol#L129

Notional x Index Coop - catchup's results

A collaboration between Notional and Index Coop to create fixed rate yield index tokens.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-47] QA Report - $88.16

Findings Information

Awards

Labels

External Links

Floating pragma and different compiler versions among contracts

Missing non-zero amount checks

Lines of code

Events missing indexed fields

Lines of code

Missing natspec comments

Lines of code

Typo in the comment

Lines of code

[G-33] Gas Report - $47.60

Findings Information

Awards

Labels

External Links

caching components.length would save SLOAD's

Lines of code

Redundant initialisation with default value

Lines of code

Prefix increment/decrements are cheaper than postfix increment/decrements

Lines of code

Using != 0 is cheaper than > 0 when used on a uint in a require() statement with the optimizer enabled

Lines of code

Error strings longer than 32 characters

Lines of code

It is better to use 1-2 for binary variables rather than 0-1

Lines of code