Inconsistency in Fee Calculation in `update_fees` Function

[hats-bug-reporter[bot]]

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0x155e6094f1322438cb40a4f20ef832d05b21b7329de6541a5c2e28d288d6586e Severity: medium

Description: Description: In the current implementation of the update_fees function, the calculation of fee accumulation does not consider the owner's virtual shares (total_shares_virtual). Instead, it only considers total_shares_minted. owners shares should be considered in total shares just like get_total_shares function. This leads to the owner losing some fee.

Impact: The owner is losing some funds due to the inconsistency in fee calculation.

Revised Code File (Optional):

@@ -417,7 +417,7 @@ impl VaultData {

         // Calculate fee accumulation since last update
         if time > 0 {
-            let virtual_shares = self.total_shares_minted * self.fee_percentage as u128 / BIPS as u128;
+            let virtual_shares = (self.total_shares_minted + self.total_shares_virtual)* self.fee_percentage as u128 / BIPS as u128;
             let time_weighted_virtual_shares = virtual_shares * time as u128 / YEAR as u128;

             self.total_shares_virtual += time_weighted_virtual_shares;
@@ -432,7 +432,7 @@ impl VaultData {

         if time > 0 {
             // Calculate fee accumulation since last update
-            let virtual_shares = self.total_shares_minted * self.fee_percentage as u128 / BIPS as u128;
+            let virtual_shares = (self.total_shares_minted + self.total_shares_virtual) * self.fee_percentage as u128 / BIPS as u128;
             let time_weighted_virtual_shares = virtual_shares * time as u128 / YEAR as u128;
             self.total_shares_virtual + time_weighted_virtual_shares
         } else {

This revised code snippet ensures that the calculation of fee accumulation in update_fees takes into account both total_shares_minted and total_shares_virtual, providing a more accurate representation of the owner's shares.

[bgibers]

The fee accumulation is only from is only a percentage of minted shares

[0xmahdirostami]

total_shares_virtual are the same as total_shares_minted, they are both shares and they are used in get_shares_from_azero and get_azero_from_shares. what is the difference between them? owner has a right to withdraw them, so if the owner does not withdraw and redeem them, they are generating yield for protocol so they must be calculated in the update fee.

as i saw in all protocols, the fee is minted in the update function and used for calculations in the next update.

for example, you delete the withdraw_fees function and delete the total_shares_virtual and mint fee shares in update_fee function. in this way, all fees are used in the next update fee and the protocol works as intended.

[bgibers]

I think you're correct here. Marking as a low, since no user or protocol funds are at risk. Rather, the protocol isn't generating the intended revenue

[bmino]

Addressed in kintsu-contracts@1e8285