-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
1 changed file
with
64 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# Issue summary | ||
|
||
| Issue Number | Title | | ||
|--------------|-------------------------------------------------------------------------------------------------------------| | ||
| L-1 | Users veNFT could not get it detached from mVeNFT | | ||
| L-2 | Users are not able to detach from mVeNFT if one of the gauges gets killed| | ||
| L-3 | Users will lose their locked fund inside veNFT | | ||
| L-4 | The revived gauges could end up receiving rewards in their disabled period | | ||
|
||
|
||
# [L-1] Users veNFT could not get it detached from mVeNFT | ||
|
||
in case a mVeNFT has only two attached veNFT, when the user called `dettachFromManagedNFT()` it will revert in`_vote()` | ||
```solidity | ||
uint256 votePowerForPool = (weights_[i] * nftVotePower) / totalVotesWeight; | ||
``` | ||
if this calculation `(weights_[i] * nftVotePower)` go smaller than `totalVotesWeight` | ||
How? The `nftVotePower` will get decreases because the user detached his veNFT | ||
Let's say the current `nftVotePower` balance is 0.5e18 | ||
and mVeNFT voted for 2 pools (one is `1e18 - 1` and the other is `1 `) | ||
so the `totalVotesWeight == 1e18` | ||
Any poke would fail as `1 * 0.5e18/ 1e18` will get rounded down to 0 voting weight. and revert in this check | ||
```solidity | ||
if (votePowerForPool == 0) { | ||
revert ZeroPowerForPool(); | ||
} | ||
``` | ||
this will leave the user NFT attached until the owner of mVeNFT updates the values or new NFTs get attached | ||
To fix it If the voting weight is 0, instead of reverting, **continue** | ||
|
||
https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VoterUpgradeableV2.sol#L740 | ||
|
||
# [L-2] Users are not able to detach from mVeNFT if one of the gauges gets killed | ||
|
||
in case mVeNFT users can not detach until the `authorizedUser` updates the vote by calling `BaseManagedNFTStrategyUpgradeable.sol#vote()`. this will lead to a temporary lock on user veNFT/funds | ||
|
||
https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VoterUpgradeableV2.sol#L730-L732 | ||
|
||
# [L-3] Users will lose their locked fund inside veNFT | ||
|
||
in the function `VotingEscrowUpgradeableV2.sol#onDettachFromManagedNFT()` | ||
in case `newBalance_ > permanentTotalSupply` the last one could lose funds and not just rewards it could be a part of his locked amount for regular veNFT when he tries to call `unlockPermanent()` tx will revert he will lose all his veNFT | ||
To fix it add a similar check in `unlockPermanent()` | ||
|
||
https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VotingEscrowUpgradeableV2.sol#L547 | ||
|
||
# [L-4] The revived gauges could end up receiving rewards in their disabled period | ||
|
||
The `VoterUpgradeableV2.sol` contract has `killGauge()` that disables the gauge to prevent it from further rewards distribution, only the address with **GOVERNANCE_ROLE** role can call it. | ||
the `killGauge()` only updates three state variables | ||
|
||
To revive a previously disabled gauge (allowing it to distribute rewards again) he will use `reviveGauge()` to set `isAlive` true again | ||
```solidity | ||
File: VoterUpgradeableV2.sol | ||
251: function reviveGauge(address gauge_) external onlyRole(_GOVERNANCE_ROLE) { | ||
... | ||
255: gaugesState[gauge_].isAlive = true; | ||
``` | ||
Now, in case gauge Z get killed in Epoch_x and after multiple epoch (Epoch_x+n) gets revived | ||
in case no one call `distributeAll()` or `distribute()` for the gauge Z | ||
So, in Epoch_x+n users will vote for it, in the next distribution [this delta](https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VoterUpgradeableV2.sol#L632) `uint256 delta = index - state.index;` | ||
will be wrong because the `state.index` is from Epoch_x. this will increase the `claimable` to transfer it to the Gauge | ||
|
||
https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VoterUpgradeableV2.sol#L632 |