Canto contest - simon135's results

Execution layer for original work.

General Information

Platform: Code4rena

Start Date: 14/06/2022

Pot Size: $100,000 USDC

Total HM: 26

Participants: 59

Period: 7 days

Judge: GalloDaSballo

Total Solo HM: 9

Id: 133

League: ETH

Canto

Findings Distribution

Researcher Performance

Rank: 41/59

Findings: 2

Award: $289.53

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Canto

Findings Information

🌟 Selected for report: joestakey

Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xf15ers, 0xmint, Bronicle, Dravee, Funen, JMukesh, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tutturu, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, cccz, codexploder, cryptphi, csanuragjain, defsec, fatherOfBlocks, gzeon, hake, hansfriese, hyh, ignacio, k, nxrblsrpr, oyc_109, robee, sach1r0, saian, simon135, technicallyty, zzzitron

Awards

73.0579 USDC - $73.06

687.9945 CANTO - $111.11

Labels

bug

QA (Quality Assurance)

External Links

Link to GitHub issue

Remove dead code/commented code or emit i think you should emit it https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/WETH.sol#L31 remove the interface and make into import and add safemath to save code size too. https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L4 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L14

1. take todo out of code

https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1613

2 dont use charot in solidity can have a diffrnace version for testing and deployment and get wierd bugs

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Accountant/AccountantInterfaces.sol#L1 comptoller.sol:1 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryDelegator.sol#L1 https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/Treasury/TreasuryInterfaces.sol#L1

just implent nonReentrant from openzeplin instead to remove dead code and also instead of lock mutex.

if msg.sender is account then he get get out reentracy and attack unsecure functions

https://github.com/Plex-Engineer/lending-market/blob/755424c1f9ab3f9f0408443e6606f94e4f08a990/contracts/CNote.sol#L351

use safemint

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L254 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L249

typos

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L173

// subtraction overflow is desired

   this commnet is not true because it will revert
        uint timeElapsed = blockTimestamp - _blockTimestampLast;

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L182

where are the comments and natspec what do the functions and parms do

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L190 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L204

ist not uniswap v3 implemnation mint and burn function its missing mintfee and /reserve0 and resver1 when assingeing liquidation

dont use safetranfer after burning tokens what the point leave buring for buring and transfering for transfer unless address0

missing 2 step check and emit of old and what new

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L497 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L502 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L509

#0 - GalloDaSballo
2022-08-03T23:35:53Z

Remove dead code/commented code or emit i think you should emit it

## 1. take todo out of code NC

2 dont use charot in solidity can have a diffrnace version for testing and deployment and get wierd bugs

just implent nonReentrant from openzeplin instead to remove dead code and also instead of lock mutex.

Disagree, let them write their own modifier

if msg.sender is account then he get get out reentracy and attack unsecure functions

Consider writing a POC and sending a Med or higher, disputing without POC

use safemint

Disagree without an explanation

typos

This is a 2 step process <img width="385" alt="Screenshot 2022-08-04 at 01 35 29" src="https://user-images.githubusercontent.com/13383782/182730428-2d2d7ec9-308f-41de-9d61-6fa2de066146.png">

Rest Disputed

4NC

#1 - GalloDaSballo
2022-08-03T23:37:02Z

Please focus on improving the presentation of the reports:

Consistent numbering for findings
Grammatical errors
Professional language: "where are the comments and natspec what do the functions and parms do" is not professional feedback

Findings Information

🌟 Selected for report: _Adam

Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, Chom, Dravee, Fitraldys, Funen, JC, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tomio, Waze, ak1, c3phas, catchup, defsec, fatherOfBlocks, gzeon, hake, hansfriese, joestakey, k, oyc_109, rfa, robee, sach1r0, saian, simon135, ynnad

Awards

41.2642 USDC - $41.26

396.9199 CANTO - $64.10

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

#++i costs less gas compared to i++ or i += 1 ++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled. i++ increments i and returns the initial value of i. Which means: uint i = 1; i++; // == 1 but i == 2 But ++i returns the actual incremented value: uint i = 1; ++i; // == 2 and i == 2 too, so no need for a temporary variable In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2 uint256 i = 0; i < depositedTokens.values.length; i++)

also index+1 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L232

2. If a variable is not set/initialized, it is assumed to have the 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.

instances include:

3. Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc. 1 byte for character

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L86 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L88 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/WETH.sol#L29 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/WETH.sol#L96-L97

4. Use Custom Errors instead of Revert Strings to save Gas

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) Source Custom Errors in Solidity: Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them. Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries). https://github.com/code-423n4/2022-05-alchemix/blob/71abbe683dfd5c0686b7e594fb4f78a14b668d8b/contracts-full/AlchemicTokenV1.sol#L52 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L86 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/WETH.sol#L29-L32 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/WETH.sol#L96-L97

caching array.lengh in a memory using memory variable to save gas

https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L940 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1267 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1459 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1757 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1762-L1783 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L207 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L88

use multiple require instead of && to save gas

https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1320 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1836-L1837

Unchecking arithmetics operations that can’t underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: Checked or Unchecked Arithmetic. I suggest wrapping with an unchecked block: Same thing with second unchecked because total can't overflow amount cant overflow

https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1324 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1459 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1757 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1762-L1783 https://github.com/Plex-Engineer/lending-market/blob/2d423c7c3f62d65182d802deb99cc7bba4e057fd/contracts/Comptroller.sol#L1841

Mark functions as payable when users can't mistakenly send ETH

impact

Functions marked as payable are 24 gas cheaper than their counterpart (in non-payable functions, Solidity adds an extra check to ensure msg.value is zero). When users can't mistakenly send ETH to a function (as an example, when there's an onlyOwner modifier or alike), it is safe to mark it as payable

proof of concept

Functions with onlyOwner modifier that aren't payable yet: instances include: noteinsterst:139,152,165

make constant internal to save gas

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L41

use 1e18 instead of 10** 18 to save gas

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L56

Using bools 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 a bitmap instead https://github.com/Plex-Engineer/stableswap/blob/1845ca6b32a8e0efc44860e817a141c0a57e44ff/contracts/BaseV1-core.sol#L777 https://github.com/Plex-Engineer/stableswap/blob/1845ca6b32a8e0efc44860e817a141c0a57e44ff/contracts/BaseV1-core.sol#L781 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L480

use structs like this wastes gas

observations.push(Observation(block.timestamp, 0, 0)); instead put each struct propteriy assign manully https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L165

!= with uint in require statment saves gas because uint > 0

https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L303 https://github.com/Plex-Engineer/stableswap/blame/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L286 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L104 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L105

use abi.encodepacked instead of abi.encode

encode has padding which wastes gas and packed puts less than bytes32 togther to save gas and dosnt pad which in return saves gas https://github.com/Plex-Engineer/stableswap/blame/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-core.sol#L415

change struct order to save gas

bool=1byte address=20 bytes put them to get to save gas address bool address https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L59

#0 - GalloDaSballo
2022-08-04T00:43:17Z

Less than 500 gas saved

Canto contest - simon135's results

Execution layer for original work.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-30] QA Report - $184.17

Findings Information

Awards

Labels

External Links

1. take todo out of code

2 dont use charot in solidity can have a diffrnace version for testing and deployment and get wierd bugs

just implent nonReentrant from openzeplin instead to remove dead code and also instead of lock mutex.

if msg.sender is account then he get get out reentracy and attack unsecure functions

use safemint

typos

// subtraction overflow is desired

where are the comments and natspec what do the functions and parms do

ist not uniswap v3 implemnation mint and burn function its missing mintfee and /reserve0 and resver1 when assingeing liquidation

dont use safetranfer after burning tokens what the point leave buring for buring and transfering for transfer unless address0

missing 2 step check and emit of old and what new

#0 - GalloDaSballo2022-08-03T23:35:53Z

Remove dead code/commented code or emit i think you should emit it

2 dont use charot in solidity can have a diffrnace version for testing and deployment and get wierd bugs

just implent nonReentrant from openzeplin instead to remove dead code and also instead of lock mutex.

if msg.sender is account then he get get out reentracy and attack unsecure functions

use safemint

typos

#1 - GalloDaSballo2022-08-03T23:37:02Z

[G-34] Gas Report - $105.36

Findings Information

Awards

Labels

External Links

2. If a variable is not set/initialized, it is assumed to have the 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.

3. Reduce the size of error messages (Long revert Strings)

4. Use Custom Errors instead of Revert Strings to save Gas

caching array.lengh in a memory using memory variable to save gas

use multiple require instead of && to save gas

Unchecking arithmetics operations that can’t underflow/overflow

Mark functions as payable when users can't mistakenly send ETH

impact

proof of concept

make constant internal to save gas

use 1e18 instead of 10** 18 to save gas

Using bools for storage incurs overhead

use structs like this wastes gas

!= with uint in require statment saves gas because uint > 0

use abi.encodepacked instead of abi.encode

change struct order to save gas

#0 - GalloDaSballo2022-08-04T00:43:17Z

#0 - GalloDaSballo
2022-08-03T23:35:53Z

#1 - GalloDaSballo
2022-08-03T23:37:02Z

#0 - GalloDaSballo
2022-08-04T00:43:17Z