Platform: Code4rena
Start Date: 29/06/2022
Pot Size: $50,000 USDC
Total HM: 20
Participants: 133
Period: 5 days
Judge: hickuphh3
Total Solo HM: 1
Id: 142
League: ETH
Rank: 65/133
Findings: 2
Award: $68.82
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: xiaoming90
Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xNineDec, 0xSolus, 0xf15ers, 0xsanson, AmitN, Bnke0x0, BowTiedWardens, Chom, David_, ElKu, Funen, GalloDaSballo, GimelSec, Hawkeye, IllIllI, JC, JohnSmith, Kaiziron, Kenshin, Lambda, Limbooo, MadWookie, Metatron, MiloTruck, Nethermind, Picodes, ReyAdmirado, Sneakyninja0129, StErMi, TomJ, Treasure-Seeker, TrungOre, Waze, Yiko, _Adam, __141345__, antonttc, async, aysha, catchup, cccz, cryptphi, csanuragjain, danb, datapunk, defsec, delfin454000, dirk_y, doddle0x, durianSausage, exd0tpy, fatherOfBlocks, gogo, hake, hansfriese, horsefacts, hubble, itsmeSTYJ, joestakey, oyc_109, pedroais, peritoflores, rajatbeladiya, reassor, robee, rokinot, samruna, saneryee, sashik_eth, shenwilly, shung, simon135, sseefried, unforgiven, zer0dot, zzzitron
47.6468 USDC - $47.65
block.timestamp is a value that keeps growing. It's the timestamp of the EVM, so it's increasing by the seconds. checking positionExpirations[uint256(orderHash)] will throw error that "Position has expired".
require(block.timestamp < positionExpirations[uint256(orderHash)], "Position has expired");
but in this case, checking block.timestamp > positionExpirations[longPositionId] || isExercised would throw and error that should be "Must not be exercised or expired" either.
require(block.timestamp > positionExpirations[longPositionId] || isExercised, "Must be exercised or expired");
it should be check if less than "<" positionExpirations[longPositionId] || isExercised
constants can be used than using numbers1.) File : PuttyV2.sol Line.241
require(_fee < 30, "fee must be less than 3%");
2.) File : PuttyV2.sol Line.287
require(order.duration < 10_000 days, "Duration too long");
3.) File : PuttyV2.sol Line.499
feeAmount = (order.strike * fee) / 1000;
Since usually ERC20 was the first being put in order it can be changed into ERC20Asset first then ERC721Asset typehash for better code readibility and use.
// check duration is valid require(order.duration < 10_000 days, "Duration too long");
Since it was for check if order.duration less than 10_000 days, and the comment said that check duration is valid. Reason string can be changed if it wasn't intended to not valid Duration.
🌟 Selected for report: GalloDaSballo
Also found by: 0v3rf10w, 0x1f8b, 0xA5DF, 0xDjango, 0xHarry, 0xKitsune, 0xNazgul, 0xNineDec, 0xc0ffEE, 0xf15ers, 0xkatana, 0xsanson, ACai, Aymen0909, Bnke0x0, BowTiedWardens, Chom, ElKu, Fitraldys, Funen, Haruxe, Hawkeye, IllIllI, JC, JohnSmith, Kaiziron, Kenshin, Lambda, Limbooo, MadWookie, Metatron, MiloTruck, Picodes, PwnedNoMore, Randyyy, RedOneN, ReyAdmirado, Ruhum, Sm4rty, StErMi, StyxRave, TerrierLover, TomJ, Tomio, UnusualTurtle, Waze, Yiko, _Adam, __141345__, ajtra, ak1, apostle0x01, asutorufos, c3phas, cRat1st0s, catchup, codetilda, cryptphi, datapunk, defsec, delfin454000, durianSausage, exd0tpy, fatherOfBlocks, gogo, grrwahrr, hake, hansfriese, horsefacts, ignacio, jayfromthe13th, joestakey, ladboy233, m_Rassska, mektigboy, minhquanym, mrpathfindr, natzuu, oyc_109, rajatbeladiya, reassor, rfa, robee, rokinot, sach1r0, saian, sashik_eth, simon135, slywaters, swit, z3s, zeesaw, zer0dot
21.1707 USDC - $21.17
++i than i++ for cost less gasUsing i++ instead ++i for all the loops, the variable i is incremented using i++. It is known that implementation by using ++i costs less gas per iteration than i++.
/contracts/src/PuttyV2.sol#L556 for (uint256 i = 0; i < orders.length; i++) { /contracts/src/PuttyV2.sol#L611 for (uint256 i = 0; i < assets.length; i++) { /contracts/src/PuttyV2.sol#L627 for (uint256 i = 0; i < assets.length; i++) { /contracts/src/PuttyV2.sol#L637 for (uint256 i = 0; i < floorTokens.length; i++) { /contracts/src/PuttyV2.sol#L647 for (uint256 i = 0; i < assets.length; i++) /contracts/src/PuttyV2.sol#L658 for (uint256 i = 0; i < floorTokens.length; i++) { /contracts/src/PuttyV2.sol#L670 for (uint256 i = 0; i < whitelist.length; i++) { /contracts/src/PuttyV2.sol#L728 for (uint256 i = 0; i < arr.length; i++) { /contracts/src/PuttyV2.sol#L742 for (uint256 i = 0; i < arr.length; i++) {
Every reason string takes at least 32 bytes. Use short reason strings that fits in 32 bytes or it will become more expensive.
/contracts/src/PuttyV2.sol#L297 "Wrong amount of floor tokenIds" /contracts/src/PuttyV2.sol#L298 "Invalid floor tokens length" /contracts/src/PuttyV2.sol#L405 "Wrong amount of floor tokenIds" /contracts/src/PuttyV2.sol#L406 "Invalid floor tokens length" /contracts/src/PuttyV2.sol#L481 "Must be exercised or expired" /contracts/src/PuttyV2.sol#L765 "URI query for NOT_MINTED token"
= 0This implementation code can be saving more gas by removing = 0, it because If a variable was not set/initialized, it is assumed to have default value to 0
/contracts/src/PuttyV2.sol#L497 uint256 feeAmount = 0;
uint256 i = 0 into uint i for saving more gasusing this implementation can saving more gas for each loops.
/contracts/src/PuttyV2.sol#L556 for (uint256 i = 0; i < orders.length; i++) { /contracts/src/PuttyV2.sol#L611 for (uint256 i = 0; i < assets.length; i++) { /contracts/src/PuttyV2.sol#L627 for (uint256 i = 0; i < assets.length; i++) { /contracts/src/PuttyV2.sol#L637 for (uint256 i = 0; i < floorTokens.length; i++) { /contracts/src/PuttyV2.sol#L647 for (uint256 i = 0; i < assets.length; i++) /contracts/src/PuttyV2.sol#L658 for (uint256 i = 0; i < floorTokens.length; i++) { /contracts/src/PuttyV2.sol#L670 for (uint256 i = 0; i < whitelist.length; i++) { /contracts/src/PuttyV2.sol#L728 for (uint256 i = 0; i < arr.length; i++) { /contracts/src/PuttyV2.sol#L742 for (uint256 i = 0; i < arr.length; i++) {