Aura Finance contest - Hawkeye's results

The comment for approveRewardDistributor() clearly states that the mentioned function should modify approvals for a distributor, ie revoke or give permission to an address to distribute rewards but the function name demonstrates a one sided role.Therefore change the name to reflect the function's dual role.

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L204

Also, consider emitting an event to alert relevant parties when a distributor is approved or disapproved.

Check whether the old address is the same as the new address

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMerkleDrop.sol#L79

cache old address in a local variable and either use :

if(oldDao≠_newDao) {

dao=_newDao;

}

Apply the same changes to the following contracts with modification to variable names for the aforementioned functions :

AuraMerkleDrop.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMerkleDrop.sol#L106

AuraStakingProxy.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraStakingProxy.sol#L92

and consider emitting an event when the crvDepositorWrapper is changed.

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraStakingProxy.sol#L139

also, it is recommended to emit an event for this function as well.

AuraVestedEscrow.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L79

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L88

Array Length should match

If there's a missing amount for a recipient address, the mapping will not contain the amount attributable to the address nor will it be added to the totalAmount :

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L103-L104

And therefore less will be sent to the AuraVestedEscrow contract if there's a non-zero value but it was not added to the array:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L108

Use require(_recipients.length==_amount.length,”mismatch”) prior to the loop.

Similar case for distribute ()

ArbitratorVault.sol

[https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/ArbitartorVault.sol#L46

Crv balance locked for staker can be unlocked sooner

Due to an incorrect value set : https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/CrvDepositor.sol#L26

The balance will be unlocked a day earlier than it should actually be. MAXTIME is calculated using 364 days, instead of 365 days which is standard for a year.

This value is used to calculate unlock At which is subsequently stored in the state variable unlockTime. initialiseLock() and lockCurve() relies on this value. It would be imperative that the unlock time is calculated properly, as the intent is for the lock period to be a year.

A similar finding was mentioned here

https://github.com/code-423n4/2022-03-paladin-findings/issues/4

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0v3rf10w, 0x1f8b, 0x4non, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, BowTiedWardens, CertoraInc, DavidGialdi, FSchmoede, Fitraldys, Funen, GimelSec, Hawkeye, JC, Kaiziron, Kthere, MaratCerby, MiloTruck, NoamYakov, QuantumBrief, Randyyy, Ruhum, SmartSek, SooYa, Tadashi, TerrierLover, Tomio, UnusualTurtle, WatchPug, Waze, _Adam, antonttc, asutorufos, bobirichman, c3phas, catchup, csanuragjain, cthulhu_cult, defsec, delfin454000, ellahi, fatherOfBlocks, hansfriese, hyh, jayjonah8, joestakey, kenta, marcopaladin, mics, minhquanym, orion, oyc_109, reassor, rfa, robee, sach1r0, samruna, sashik_eth, sikorico, simon135, unforgiven, z3s, zmj

Awards

83.2689 USDC - $83.27

Labels

bug

duplicate

G (Gas Optimization)

External Links

Link to GitHub issue

Gas

I. pendingPenalty will be accessed unnecessarily if there's no penalty stored. To prevent continuous access of the state variable and it being updated to zero(already the default), check whether there's any penalty before resetting to zero.

AuraBalRewardPool.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraBalRewardPool.sol#L197

Add the following lines to forwardPenalty() :

if (toForward>0){

 pendingPenalty = 0;
     }

The same applies for :

AuraMerkleDrop.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMerkleDrop.sol#L151

II. epochIndex already caches the length of the epoch. Therefore, replace all reference to the state variable below with the local variable:

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraLocker.sol#L334-L335

III.Utilise unchecked block to save gas

The calculation for penalty will always be the smallest of the two operands :

AuraMerkleDrop.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraMerkleDrop.sol#L138

AuraBalRewardPool.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraBalRewardPool.sol#L185

Remove AuraMath.sub() as the condition necessitates that rdata.periodFinish is the larger operand and will not underflow:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L866

AuraVestedEscrow.sol

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L162

Aura Finance contest - Hawkeye's results

Providing optimal incentives for VotingEscrow systems.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-33] QA Report - $170.37

Findings Information

Awards

Labels

External Links

QA

Grammatical Errors

NatSpec tag missing

Constants should follow style guide

Use mixedCase for Immutable state variables

State Variables are already their default types

Comment doesn't reflect the code

Function names should be unambiguous

Array Length should match

Crv balance locked for staker can be unlocked sooner

[G-28] Gas Report - $83.27

Findings Information

Awards

Labels

External Links

Gas