Description

The costSharePrice is used in the VaultV2 and WooSuperChargerVaultV2 contracts to calculate the user's share price when depositing funds. The costSharePrice is calculated as the weighted average share price when the user deposits the funds.

It is calculated upon depositing funds, where the user is issued new shares. It is, however, not updated when the user transfers their shares to another user. This means that the costSharePrice is not properly maintained when the user transfers their shares to another user.

If User A has never called deposit, having their costSharePrice = 0 while User B has called deposit (so User B's costSharePrice > 0) and then manually transfers their shares to User A, then User A will have the balance > 0 but their costPerSharePrice will still be zero.

Impact

The impact is strictly related to user experience at the moment, as the costSharePrice is used for displaying purposes only. For posterity, however, we note that proper accounting is a good practice to follow, as it can prevent future issues.

Recommendations

We recommend properly accounting for the transfer case, where the costSharePrice is not properly maintained.

Remediation

This issue has been acknowledged by WOOFI, who noted the following:

CostSharePrice couldn't handle the transfer case; We knew it and costPerSharePrice is used for displaying purposes. Vault v1 doesn't have this field, only after a few power users want to see their PnL for each vault, then we considered adding this field in V2.

WOOFI states:

Currently known issue; it's only for displaying purposes, and we're noticing users in website that transferring the WE token may cause the PNL miscalculated.