dHEDGE - Ⅳ

In `DhedgeStakingV2.sol` anyone can call `_addToGlobalStake()` and `_removeFromGlobalStake()`. Thus the variables `dhtStaked` and `aggregateStakeStartTime` can be altered by the caller without limitations.

When computing the DHT reward amount, through calling `calculateDhtRewardAmount()`, the current values of RewardParams struct are used except for `EmissionsRate` numerator, which is stored inside the Stake struct itself and so the value used for calculations is the one saved during `_newStake()`. Because 5 of the 7 attributes of the RewardParams struct can change due to setter functions, old and current values are used during calculations. Users must be aware that the values used in reward calculation process may not be those from when the new stake was created. Therefore, for example, a user who creates a stake when `stakeDurationDelaySeconds` = 30 days expects to receive a certain `stakeDurationMultiplier` that may be far different if `stakeDurationDelaySeconds` has been changed in the meantime. ```solidity struct RewardParams { uint256 stakeDurationDelaySeconds; //can change uint256 maxDurationBoostSeconds; //can change uint256 maxPerformanceBoostNumerator; //can change uint256 maxPerformanceBoostDenominator; uint256 stakingRatio; //can change uint256 emissionsRate; //can change uint256 emissionsRateDenominator; } ```

Using strict equality to check a volatile numeric value such as Ether/token balance, time, and block number is risky, because such a value can be manipulated by an attacker. In particular in `_removeFromGlobalStake()`, `target` and `dhtStaked` variables, which are both volatile numbers, are compared: ```solidity=265 if (target == dhtStaked) { aggregateStakeStartTime = block.timestamp.sub(maxVDurationTimeSeconds); } ```

Performing integer division before multiplication truncates the low bits, losing the precision of calculation. ```solidity=83 return (maxVDHTAllowed).mul(performanceFactor).div(UNIT).mul(stakeDurationFactor).div(UNIT).mul(stakeEmissionsRate).div( rewardParams.emissionsRateDenominator ); ```

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

In `_checkMaxStaking()` function the following require statement is present: ```solidity=476 require(totalStakingValue <= maxStakingValue(), "Staking cap will be exceeded."); ``` It seems ineffective because totalStakingValue is a smaller value than maxStakingValue by several orders of magnitude. In particular: - `totalStakingValue` represents the value of the amount of DHPT that have been staked so far not considering decimals places. Indeed before adding values to `totalStakingValue` they are divided by 10**18 as done for `originalValueStaked` and `totalDepositValue`; - `maxStakingValue()` returns the value of `dhtCap` which consider decimals after some calculations not involving the decimals.

`performanceMultiplier` and `stakeDurationMultiplier` are retrieved respectively from `calculatePerformanceMultiplier()` and `calculateStakeDurationMultiplier()`. Their value could be in the following range: [0 ; `UNIT`]. Therefore when computing the return value of `calculateDhtRewardAmount()`, `maxVDHTAllowed` is multiplied to those two factors: - `performanceMultiplier` / `UNIT`, which is <= 1; - `stakeDurationMultiplier` / `UNIT`, which is <= 1; So the value of `maxVDHTAllowed` at most can remain unchanged (not considering subsequent calculations) and therefore the names `performanceMultiplier` and `stakeDurationMultiplier` are misleading since they do not bring in any increase in that value in any case.

In `DhedgeStakingV2Storage.sol` there are no lower bound and upper bound checks for the following input parameters, which can then be set to any value: - `newMaxVDurationTimeSeconds` in `setMaxVDurationTimeSeconds()`; - `newStakeDurationDelaySeconds` in `setStakeDurationDelaySeconds()`; - `newMaxDurationBoostSeconds` in `setMaxDurationBoostSeconds()`; - `newMaxPerformanceBoostNumerator` in `setMaxPerformanceBoostNumerator()`; - `newStakingRatio` in `setStakingRatio()`; - `newEmissionsRate` in `setEmissionsRate()`; - `newDHTCap` in `setDHTCap()`; - `newRewardStreamingTime` in `setRewardStreamingTime()`;

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. It is possible to reenter the function `newStake()` as it will call `_safeMint()`, which makes the external call `_checkOnERC721Received()` that can be coded to reenter the function. If the function is reentered it can cause the `NewStake` events to be emitted out of order. Similarly, it is possible to reenter the function `unstakePoolTokens()` as it will call `_safeMint()`, which makes the external call `_checkOnERC721Received()` that can be coded to reenter the function. In this case it can emit the `unstakePoolTokens` events out of order.