Medium Risk
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXVault.sol#L334
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXVault.sol#L615
The protocol charges a management fee based on the feePerSecond
variable, which dictates the rate at which new vault tokens are minted as fees via the mintFee
function. An administrative function updateFeePerSecond
allows the owner to alter this fee rate. However, the current implementation does not account for accrued fees before the update, potentially leading to incorrect fee calculation.
The contract's logic fails to account for outstanding fees at the old rate prior to updating the feePerSecond
. As it stands, the updateFeePerSecond
function changes the fee rate without triggering a mintFee
, which would update the lastFeeCollected
timestamp and mint the correct amount of fees owed up until that point.
function updateFeePerSecond(uint256 feePerSecond) external onlyOwner {
_store.feePerSecond = feePerSecond;
emit FeePerSecondUpdated(feePerSecond);
}
Scenario Illustration:
mintFee
and setting lastFeeCollected
to the current block.timestamp
.mintFee
calls occur.updateFeePerSecond
to increase the fee by 10%.mintFee
now calculates fees since lastFeeCollected
using the new, higher rate, incorrectly applying it to the period before the rate change.The impact is twofold:
feePerSecond
results in excessively high fees charged for the period before the update.feePerSecond
leads to lower-than-expected fees for the same duration.Manual Analysis
Ensure the fees are accurately accounted for at their respective rates by updating lastFeeCollected
to the current timestamp prior to altering the feePerSecond
. This can be achieved by invoking mintFee
within the updateFeePerSecond
function to settle all pending fees first:
function updateFeePerSecond(uint256 feePerSecond) external onlyOwner {
self.vault.mintFee();
_store.feePerSecond = feePerSecond;
emit FeePerSecondUpdated(feePerSecond);
}