Contract Updates

The contract currently calculates total staked based on the contract’s IPRWA balance. When royalties are deposited, they inflate the staking balance, effectively treating rewards as staked funds. This causes overestimation of staking size and dilution of later rewards.

Maintain a dedicated totalStaked state variable updated only during stake/unstake actions.

In the current design, the distribute function is called after the unstake action. This means users receive IPRWA based on the previous snapshot, potentially under-allocating yield since they were contributing until the point of unstake. While this was intended to avoid gaming the system, it still leaves gaps: for example, a user can repeatedly stake as little as 1 wei to trigger distribution first and then immediately unstake, gaining advantage from timing.

Update distributions before calculating shares-to-assets conversion in the unstake function. This ensures users are attributed yield for the full duration of their stake and avoids unnecessary edge cases where minimal stakes can influence the distribution order.

The protocol introduces randomness (interval + delta) to determine whether distributions occur. While the intent is to mitigate MEV at boundaries, this randomness can be bypassed by wrapper contracts that deterministically call stake only when distribution would be skipped.

Randomness could also result in one set of users not getting attributed to their yield, while one set is.

It also requires consideration of forceDistribute by admin.

Hence, the probabilistic design increases complexity without clear security gains.

Consider streaming royalties proportionally based on elapsed time since the last distribution. For example, distribute (elapsed / year) * yearlyDistribution. This approach:

• Eliminates exploitable randomness,
• Reduces unnecessary complexity,
• Ensures smooth, continuous distribution,
• Aligns directly with stake/unstake actions already in place.

The initialize() function of RoyaltiesHook only calls __ReentrancyGuard_init(), but does not invoke __AccessControl_init() or __UUPSUpgradeable_init().

Currently, these functions are empty in the OpenZeppelin 0.8.x implementation, so the contract will work as expected. However, skipping them breaks the standard upgradeable-contract initialization pattern and may cause problems if OpenZeppelin adds initialization logic in future versions or if multiple inheritance chains rely on proper linearization.

Call all inherited initializer functions inside initialize() to preserve upgrade-safe patterns and maintain forward compatibility:

__AccessControl_init();
__ReentrancyGuard_init();
__UUPSUpgradeable_init();

The current comment suggests that avgAPR is used to calculate the total amount of royalties to be distributed.

avgAPR is used to calculate the total amount of royalties to be distributed, whereas

This is incorrect.

• APR represents the intended annual reward rate (e.g., 10%).
• avgAPR represents how much has already been rewarded (but not yet claimed), relative to the staked balance.

For example:

• Initial stake = 1000 tokens, APR = 10%.
• Expected yearly rewards = 100 tokens.
• After one month: 100 ÷ 12 = 8.33 tokens accrued.
• avgAPR at this point = (8.33 ÷ 1000) × 100% ≈ 0.833%.

If a user claims rewards, avgAPR must decrease accordingly. Therefore, avgAPR must always be adjusted as:

avgAPR= (rewards accrued − claimed)/stake ×100%

Calculate avgAPR considering above and ensure that avgAPR is consistently adjusted after claims, so it accurately reflects outstanding (unclaimed) rewards.

Also consider correcting comment in contract to reflect actual nature of avgAPR.

Direct donations increase stakedAndRewards but not approxRewards. While normally unattractive (since donors effectively give value to others), this creates a potential exploit during the vault’s initial phase.

Attack Scenario

1. A malicious actor deposits 1 wei as the first stake.
2. They wait for time to pass.
3. Instead of staking again (which updates lastDistributionTime), they donate directly.
4. The system incorrectly treats this donation as if it had been staked since the beginning, releasing more rewards than intended.

If the first few depositors collude, they could inflate rewards early on at minimal cost.

On redeployment or when vault doesn't have much deposits, consider making the first substantial deposit to prevent exploitation by early depositors.

The IRoyaltiesHookAdmin interface currently lacks functions for updating key parameters:

• APR (the target annual reward rate)
• avgAPR (the effective reward rate already accrued but not yet claimed)

Consider extending IRoyaltiesHookAdmin to include:

• setAPR(uint256 newAPR)
• setAvgAPR(uint256 newAvgAPR)