The implied value of a public vault can be impaired, liquidity providers can lose funds
The implied value of a public vault can be impaired, liquidity providers can lose funds\\nBorrowers can partially repay their liens, which is handled by the `_payment` function (LienToken.sol#L594). When repaying a part of a lien, `lien.amount` is updated to include currently accrued debt (LienToken.sol#L605-L617):\\n```\\nLien storage lien = lienData[lienId];\\nlien.amount = _getOwed(lien); // @audit current debt, including accrued interest; saved to storage!\\n```\\n\\nNotice that `lien.amount` is updated in storage, and `lien.last` wasn't updated.\\nThen, lien's slope is subtracted from vault's slope accumulator to be re-calculated after the repayment (LienToken.sol#L620-L630):\\n```\\nif (isPublicVault) {\\n  // @audit calculates and subtracts lien's slope from vault's slope\\n  IPublicVault(lienOwner).beforePayment(lienId, paymentAmount);\\n}\\nif (lien.amount > paymentAmount) {\\n  lien.amount -= paymentAmount;\\n  // @audit lien.last is updated only after payment amount subtraction\\n  lien.last = block.timestamp.safeCastTo32();\\n  // slope does not need to be updated if paying off the rest, since we neutralize slope in beforePayment()\\n  if (isPublicVault) {\\n    // @audit re-calculates and re-applies lien's slope after the repayment\\n    IPublicVault(lienOwner).afterPayment(lienId);\\n  }\\n}\\n```\\n\\nIn the `beforePayment` function, `LIEN_TOKEN().calculateSlope(lienId)` is called to calculate lien's current slope (PublicVault.sol#L433-L442):\\n```\\nfunction beforePayment(uint256 lienId, uint256 amount) public onlyLienToken {\\n  _handleStrategistInterestReward(lienId, amount);\\n  uint256 lienSlope = LIEN_TOKEN().calculateSlope(lienId);\\n  if (lienSlope > slope) {\\n    slope = 0;\\n  } else {\\n    slope -= lienSlope;\\n  }\\n  last = block.timestamp;\\n}\\n```\\n\\nThe `calculateSlope` function reads a lien from storage and calls `_getOwed` again (LienToken.sol#L440-L445):\\n```\\nfunction calculateSlope(uint256 lienId) public view returns (uint256) {\\n  // @audit lien.amount includes interest accrued so far\\n  Lien memory lien = lienData[lienId];\\n  uint256 end = (lien.start + lien.duration);\\n  uint256 owedAtEnd = _getOwed(lien, end);\\n  // @audit lien.last wasn't updated in `_payment`, it's an older timestamp\\n  return (owedAtEnd - lien.amount).mulDivDown(1, end - lien.last);\\n}\\n```\\n\\nThis is where double counting of accrued interest happens. Recall that lien's amount already includes the interest that was accrued by this moment (in the `_payment` function). Now, interest is calculated again and is applied to the amount that already includes (a portion) it (LienToken.sol#L544-L550):\\n```\\nfunction _getOwed(Lien memory lien, uint256 timestamp)\\n  internal\\n  view\\n  returns (uint256)\\n{\\n  // @audit lien.amount already includes interest accrued so far\\n  return lien.amount + _getInterest(lien, timestamp);\\n}\\n```\\n\\nLienToken.sol#L177-L196:\\n```\\nfunction _getInterest(Lien memory lien, uint256 timestamp)\\n  internal\\n  view\\n  returns (uint256)\\n{\\n  if (!lien.active) {\\n    return uint256(0);\\n  }\\n  uint256 delta_t;\\n  if (block.timestamp >= lien.start + lien.duration) {\\n    delta_t = uint256(lien.start + lien.duration - lien.last);\\n  } else {\\n    // @audit lien.last wasn't updated in `_payment`, so the `delta_t` is bigger here\\n    delta_t = uint256(timestamp.safeCastTo32() - lien.last);\\n  }\\n  return\\n    // @audit rate applied to a longer delta_t and multiplied by a bigger amount than expected\\n    delta_t.mulDivDown(lien.rate, 1).mulDivDown(\\n      lien.amount,\\n      INTEREST_DENOMINATOR\\n    );\\n}\\n```\\n
In the `_payment` function, consider updating `lien.amount` after the `beforePayment` call:\\n```\\n// Remove the line below\\n// Remove the line below\\n// Remove the line below\\n a/src/LienToken.sol\\n// Add the line below\\n// Add the line below\\n// Add the line below\\n b/src/LienToken.sol\\n@@ // Remove the line below\\n614,12 // Add the line below\\n614,13 @@ contract LienToken is ERC721, ILienToken, Auth, TransferAgent {\\n       type(IPublicVault).interfaceId\\n     );\\n\\n// Remove the line below\\n    lien.amount = _getOwed(lien);\\n// Remove the line below\\n\\n     address payee = getPayee(lienId);\\n     if (isPublicVault) {\\n       IPublicVault(lienOwner).beforePayment(lienId, paymentAmount);\\n     }\\n// Add the line below\\n\\n// Add the line below\\n    lien.amount = _getOwed(lien);\\n// Add the line below\\n\\n     if (lien.amount > paymentAmount) {\\n       lien.amount // Remove the line below\\n= paymentAmount;\\n       lien.last = block.timestamp.safeCastTo32();\\n```\\n\\nIn this case, lien's slope calculation won't be affected in the `beforePayment` call and the correct slope will be removed from the slope accumulator.
Double counting of interest will result in a wrong lien slope, which will affect the vault's slope accumulator. This will result in an invalid implied value of a vault (PublicVault.sol#L406-L413):\\nIf miscalculated lien slope is bigger than expected, vault's slope will be smaller than expected (due to the subtraction in beforePayment), and vault's implied value will also be smaller. Liquidity providers will lose money because they won't be able to redeem the whole liquidity (vault's implied value, `totalAssets`, is used in the conversion of LP shares, ERC4626-Cloned.sol#L392-L412)\\nIf miscalculated lien slope is smaller than expected, vault's slope will be higher, and vaults implied value will also be higher. However, it won't be backed by actual liquidity, thus the liquidity providers that exit earlier will get a bigger share of the underlying assets. The last liquidity provider won't be able to get their entire share.
```\\nLien storage lien = lienData[lienId];\\nlien.amount = _getOwed(lien); // @audit current debt, including accrued interest; saved to storage!\\n```\\n