Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fee questions #34

Closed
MerlinEgalite opened this issue Sep 11, 2023 · 1 comment · Fixed by #43
Closed

Fee questions #34

MerlinEgalite opened this issue Sep 11, 2023 · 1 comment · Fixed by #43
Assignees
Labels
🧐 question Further information is requested

Comments

@MerlinEgalite
Copy link
Contributor

MerlinEgalite commented Sep 11, 2023

I have some questions about the fee mechanism:

  1. When setting a fee, should revert if the fee recipient is not set? and also do the opposite, revert if there's a fee but we try to remove the fee recipient?
  2. A somehow related question. On fee accrual we skip the update if the fee recipient is zero. I feel that we should skip only if fee is 0.
    https://github.com/morpho-labs/morpho-blue-metamorpho/blob/4c7b5d69bc9d2e2a8eac801e30c0b53af3906388/contracts/SupplyVault.sol#L459
    Else, we could set a fee and a fee recipient, updating lastTotalAssets, remove the fee recipient so that lastTotalAssets is not updated for a while. Then, add a new fee recipient so that lastTotalAssets is the totalInterest generated is very (very) big. This needs to be clarified on my side but I think the fee recipient could basically steal users' funds.
@MerlinEgalite MerlinEgalite added the 🧐 question Further information is requested label Sep 11, 2023
@MerlinEgalite MerlinEgalite self-assigned this Sep 11, 2023
@Rubilmax
Copy link
Contributor

Rubilmax commented Sep 12, 2023

  1. agreed
  2. makes sense to me only if 1 is implemented (because it requires the invariant fee != 0 => feeRecipient != 0)
  3. fixed in Fix deposit fee #38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧐 question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants