Platform: Code4rena
Start Date: 12/08/2022
Pot Size: $35,000 USDC
Total HM: 10
Participants: 126
Period: 3 days
Judge: Justin Goro
Total Solo HM: 3
Id: 154
League: ETH
Rank: 111/126
Findings: 1
Award: $14.96
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x040, 0x1f8b, 0xDjango, 0xHarry, 0xLovesleep, 0xNazgul, 0xNineDec, 0xSmartContract, 0xackermann, 0xbepresent, 2997ms, Amithuddar, Aymen0909, Bnke0x0, CRYP70, CertoraInc, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, Fitraldys, Funen, GalloDaSballo, JC, JohnSmith, Junnon, LeoS, Metatron, MiloTruck, Noah3o6, NoamYakov, PaludoX0, RedOneN, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, SooYa, SpaceCake, TomJ, Tomio, Waze, Yiko, __141345__, a12jmx, ajtra, ak1, apostle0x01, asutorufos, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, chrisdior4, csanuragjain, d3e4, defsec, delfin454000, djxploit, durianSausage, ellahi, erictee, fatherOfBlocks, gerdusx, gogo, ignacio, jag, ladboy233, m_Rassska, medikko, mics, natzuu, newfork01, oyc_109, paribus, pfapostol, rbserver, reassor, ret2basic, robee, rokinot, rvierdiiev, sach1r0, saian, sashik_eth, sikorico, simon135
14.9558 USDC - $14.96
https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol
diff --git a/contracts/VotingEscrow.sol b/contracts/VotingEscrow.sol index f15781a..4ece9a8 100644 --- a/contracts/VotingEscrow.sol +++ b/contracts/VotingEscrow.sol @@ -211,7 +211,8 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { if (uepoch == 0) { return (0, 0, 0); } - Point memory point = userPointHistory[_addr][uepoch]; + //Use storage reference, since storage reads are less than "copy from storage to memory" + Point storage point = userPointHistory[_addr][uepoch]; return (point.bias, point.slope, point.ts); } @@ -253,15 +254,17 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { // Moved from bottom final if statement to resolve stack too deep err // start { // Now handle user history - uint256 uEpoch = userPointEpoch[_addr]; - if (uEpoch == 0) { - userPointHistory[_addr][uEpoch + 1] = userOldPoint; - } + unchecked { + uint256 uEpoch = userPointEpoch[_addr]; + if (uEpoch == 0) { + userPointHistory[_addr][uEpoch + 1] = userOldPoint; + } - userPointEpoch[_addr] = uEpoch + 1; - userNewPoint.ts = block.timestamp; - userNewPoint.blk = block.number; - userPointHistory[_addr][uEpoch + 1] = userNewPoint; + userPointEpoch[_addr] = uEpoch + 1; + userNewPoint.ts = block.timestamp; + userNewPoint.blk = block.number; + userPointHistory[_addr][uEpoch + 1] = userNewPoint; + } // } end @@ -297,19 +300,23 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { Point({ bias: 0, slope: 0, ts: lastPoint.ts, blk: lastPoint.blk }); uint256 blockSlope = 0; // dblock/dt if (block.timestamp > lastPoint.ts) { - blockSlope = - (MULTIPLIER * (block.number - lastPoint.blk)) / - (block.timestamp - lastPoint.ts); + unchecked { + blockSlope = + (MULTIPLIER * (block.number - lastPoint.blk)) / + (block.timestamp - lastPoint.ts); + } } // If last point is already recorded in this block, slope=0 // But that's ok b/c we know the block in such case // Go over weeks to fill history and calculate what the current point is uint256 iterativeTime = _floorToWeek(lastCheckpoint); - for (uint256 i = 0; i < 255; i++) { + for (uint256 i; i < 255;) { // Hopefully it won't happen that this won't get used in 5 years! // If it does, users will be able to withdraw but vote weight will be broken - iterativeTime = iterativeTime + WEEK; + unchecked { + iterativeTime = iterativeTime + WEEK; + } int128 dSlope = 0; if (iterativeTime > block.timestamp) { iterativeTime = block.timestamp; @@ -337,13 +344,19 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { MULTIPLIER; // when epoch is incremented, we either push here or after slopes updated below - epoch = epoch + 1; + unchecked { + ++epoch; + } if (iterativeTime == block.timestamp) { lastPoint.blk = block.number; break; } else { pointHistory[epoch] = lastPoint; } + + unchecked { + ++i; + } } globalEpoch = epoch; @@ -375,20 +388,22 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { // Schedule the slope changes (slope is going down) // We subtract new_user_slope from [new_locked.end] // and add old_user_slope to [old_locked.end] - if (_oldLocked.end > block.timestamp) { - // oldSlopeDelta was <something> - userOldPoint.slope, so we cancel that - oldSlopeDelta = oldSlopeDelta + userOldPoint.slope; - if (_newLocked.end == _oldLocked.end) { - oldSlopeDelta = oldSlopeDelta - userNewPoint.slope; // It was a new deposit, not extension + unchecked { + if (_oldLocked.end > block.timestamp) { + // oldSlopeDelta was <something> - userOldPoint.slope, so we cancel that + oldSlopeDelta = oldSlopeDelta + userOldPoint.slope; + if (_newLocked.end == _oldLocked.end) { + oldSlopeDelta = oldSlopeDelta - userNewPoint.slope; // It was a new deposit, not extension + } + slopeChanges[_oldLocked.end] = oldSlopeDelta; } - slopeChanges[_oldLocked.end] = oldSlopeDelta; - } - if (_newLocked.end > block.timestamp) { - if (_newLocked.end > _oldLocked.end) { - newSlopeDelta = newSlopeDelta - userNewPoint.slope; // old slope disappeared at this point - slopeChanges[_newLocked.end] = newSlopeDelta; + if (_newLocked.end > block.timestamp) { + if (_newLocked.end > _oldLocked.end) { + newSlopeDelta = newSlopeDelta - userNewPoint.slope; // old slope disappeared at this point + slopeChanges[_newLocked.end] = newSlopeDelta; + } + // else: we recorded it already in oldSlopeDelta } - // else: we recorded it already in oldSlopeDelta } } } @@ -600,7 +615,9 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { ) internal { LockedBalance memory newLocked = _copyLock(_locked); if (action == LockAction.DELEGATE) { - newLocked.delegated += value; + unchecked { + newLocked.delegated += value; + } emit Deposit( addr, uint256(int256(value)), @@ -609,7 +626,9 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { block.timestamp ); } else { - newLocked.delegated -= value; + unchecked { + newLocked.delegated -= value; + } emit Withdraw( addr, uint256(int256(value)), @@ -711,10 +730,10 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { returns (uint256) { // Binary search - uint256 min = 0; + uint256 min; uint256 max = _maxEpoch; // Will be always enough for 128-bit numbers - for (uint256 i = 0; i < 128; i++) { + for (uint256 i; i < 128;) { if (min >= max) break; uint256 mid = (min + max + 1) / 2; if (pointHistory[mid].blk <= _block) { @@ -722,6 +741,10 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { } else { max = mid - 1; } + + unchecked { + ++i; + } } return min; } @@ -734,9 +757,9 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { view returns (uint256) { - uint256 min = 0; + uint256 min; uint256 max = userPointEpoch[_addr]; - for (uint256 i = 0; i < 128; i++) { + for (uint256 i; i < 128;) { if (min >= max) { break; } @@ -746,6 +769,10 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { } else { max = mid - 1; } + + unchecked { + ++i; + } } return min; } @@ -790,8 +817,8 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { // Calculate delta (block & time) between user Point and target block // Allowing us to calculate the average seconds per block between // the two points - uint256 dBlock = 0; - uint256 dTime = 0; + uint256 dBlock; + uint256 dTime; if (epoch < maxEpoch) { Point memory point1 = pointHistory[epoch + 1]; dBlock = point1.blk - point0.blk; @@ -831,8 +858,10 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { // Floor the timestamp to weekly interval uint256 iterativeTime = _floorToWeek(lastPoint.ts); // Iterate through all weeks between _point & _t to account for slope changes - for (uint256 i = 0; i < 255; i++) { - iterativeTime = iterativeTime + WEEK; + for (uint256 i; i < 255;) { + unchecked { + iterativeTime = iterativeTime + WEEK; + } int128 dSlope = 0; // If week end is after timestamp, then truncate & leave dSlope to 0 if (iterativeTime > _t) { @@ -852,6 +881,10 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { } lastPoint.slope = lastPoint.slope + dSlope; lastPoint.ts = iterativeTime; + + unchecked { + ++i; + } } if (lastPoint.bias < 0) { @@ -886,7 +919,7 @@ contract VotingEscrow is IVotingEscrow, ReentrancyGuard { return 0; } - uint256 dTime = 0; + uint256 dTime; if (targetEpoch < epoch) { Point memory pointNext = pointHistory[targetEpoch + 1]; if (point.blk != pointNext.blk) {
Original vs Optimized gas diff
diff --git a/gas.txt b/gas.txt index a014cff..6a4a2ed 100644 --- a/gas.txt +++ b/gas.txt @@ -1,4 +1,3 @@ - ·------------------------------------------|---------------------------|---------------|-----------------------------· | Solc version: 0.7.1 · Optimizer enabled: true · Runs: 10000 · Block limit: 12450000 gas │ ···········································|···························|···············|······························ @@ -6,7 +5,7 @@ ····················|······················|·············|·············|···············|···············|·············· | Contract · Method · Min · Max · Avg · # calls · eur (avg) │ ····················|······················|·············|·············|···············|···············|·············· -| Blocklist · block · 45000 · 409628 · 198815 · 5 · - │ +| Blocklist · block · 45000 · 407947 · 198143 · 5 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockERC20 · approve · 46176 · 46200 · 46196 · 96 · - │ ····················|······················|·············|·············|···············|···············|·············· @@ -16,35 +15,35 @@ ····················|······················|·············|·············|···············|···············|·············· | MockERC20 · transfer · 51588 · 51612 · 51606 · 83 · - │ ····················|······················|·············|·············|···············|···············|·············· -| MockSmartWallet · createLock · 334002 · 378450 · 355494 · 7 · - │ +| MockSmartWallet · createLock · 333318 · 378007 · 354913 · 7 · - │ ····················|······················|·············|·············|···············|···············|·············· -| MockSmartWallet · delegate · - · - · 340388 · 2 · - │ +| MockSmartWallet · delegate · - · - · 338707 · 2 · - │ ····················|······················|·············|·············|···············|···············|·············· -| MockSmartWallet · increaseUnlockTime · - · - · 208859 · 1 · - │ +| MockSmartWallet · increaseUnlockTime · - · - · 208060 · 1 · - │ ····················|······················|·············|·············|···············|···············|·············· -| MockSmartWallet · quitLock · 131158 · 256210 · 201886 · 4 · - │ +| MockSmartWallet · quitLock · 130516 · 255373 · 201147 · 4 · - │ ····················|······················|·············|·············|···············|···············|·············· -| MockSmartWallet · withdraw · - · - · 666280 · 1 · - │ +| MockSmartWallet · withdraw · - · - · 663842 · 1 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · checkpoint · 82307 · 3715511 · 1272491 · 10 · - │ +| VotingEscrow · checkpoint · 81945 · 3705009 · 1268853 · 10 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · collectPenalty · - · - · 49948 · 1 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · createLock · 293060 · 3978208 · 414348 · 60 · - │ +| VotingEscrow · createLock · 292437 · 3967384 · 413523 · 60 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · delegate · 246709 · 4048411 · 650225 · 33 · - │ +| VotingEscrow · delegate · 245586 · 4036706 · 647799 · 33 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · increaseAmount · 234777 · 1077801 · 448554 · 4 · - │ +| VotingEscrow · increaseAmount · 233978 · 1074662 · 447170 · 4 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · increaseUnlockTime · 46794 · 416024 · 143186 · 23 · - │ +| VotingEscrow · increaseUnlockTime · 46794 · 414640 · 142762 · 23 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · quitLock · 127639 · 1605731 · 375486 · 13 · - │ +| VotingEscrow · quitLock · 126997 · 1600994 · 374244 · 13 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · unlock · - · - · 14596 · 3 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · updateBlocklist · - · - · 47186 · 14 · - │ ····················|······················|·············|·············|···············|···············|·············· -| VotingEscrow · withdraw · 106462 · 3752666 · 610141 · 33 · - │ +| VotingEscrow · withdraw · 105974 · 3742038 · 608276 · 33 · - │ ····················|······················|·············|·············|···············|···············|·············· | Deployments · · % of limit · │ ···········································|·············|·············|···············|···············|·············· @@ -54,5 +53,5 @@ ···········································|·············|·············|···············|···············|·············· | MockSmartWallet · - · - · 416156 · 3.3 % · - │ ···········································|·············|·············|···············|···············|·············· -| VotingEscrow · 4374338 · 4374350 · 4374350 · 35.1 % · - │ +| VotingEscrow · 4322000 · 4322012 · 4322012 · 34.7 % · - │ ·------------------------------------------|-------------|-------------|---------------|---------------|-------------·
Gas report with optimization
·------------------------------------------|---------------------------|---------------|-----------------------------· | Solc version: 0.7.1 · Optimizer enabled: true · Runs: 10000 · Block limit: 12450000 gas │ ···········································|···························|···············|······························ | Methods │ ····················|······················|·············|·············|···············|···············|·············· | Contract · Method · Min · Max · Avg · # calls · eur (avg) │ ····················|······················|·············|·············|···············|···············|·············· | Blocklist · block · 45000 · 407947 · 198143 · 5 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockERC20 · approve · 46176 · 46200 · 46196 · 96 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockERC20 · mint · 53437 · 70585 · 64871 · 21 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockERC20 · setAllowance · 45021 · 45033 · 45031 · 5 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockERC20 · transfer · 51588 · 51612 · 51606 · 83 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockSmartWallet · createLock · 333318 · 378007 · 354913 · 7 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockSmartWallet · delegate · - · - · 338707 · 2 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockSmartWallet · increaseUnlockTime · - · - · 208060 · 1 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockSmartWallet · quitLock · 130516 · 255373 · 201147 · 4 · - │ ····················|······················|·············|·············|···············|···············|·············· | MockSmartWallet · withdraw · - · - · 663842 · 1 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · checkpoint · 81945 · 3705009 · 1268853 · 10 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · collectPenalty · - · - · 49948 · 1 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · createLock · 292437 · 3967384 · 413523 · 60 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · delegate · 245586 · 4036706 · 647799 · 33 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · increaseAmount · 233978 · 1074662 · 447170 · 4 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · increaseUnlockTime · 46794 · 414640 · 142762 · 23 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · quitLock · 126997 · 1600994 · 374244 · 13 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · unlock · - · - · 14596 · 3 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · updateBlocklist · - · - · 47186 · 14 · - │ ····················|······················|·············|·············|···············|···············|·············· | VotingEscrow · withdraw · 105974 · 3742038 · 608276 · 33 · - │ ····················|······················|·············|·············|···············|···············|·············· | Deployments · · % of limit · │ ···········································|·············|·············|···············|···············|·············· | Blocklist · 278212 · 278236 · 278231 · 2.2 % · - │ ···········································|·············|·············|···············|···············|·············· | MockERC20 · - · - · 1278169 · 10.3 % · - │ ···········································|·············|·············|···············|···············|·············· | MockSmartWallet · - · - · 416156 · 3.3 % · - │ ···········································|·············|·············|···············|···············|·············· | VotingEscrow · 4322000 · 4322012 · 4322012 · 34.7 % · - │ ·------------------------------------------|-------------|-------------|---------------|---------------|-------------·