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

Add prorated fees for open term loans #79

Merged
merged 8 commits into from
Nov 9, 2022
Merged

Add prorated fees for open term loans #79

merged 8 commits into from
Nov 9, 2022

Conversation

bricestacey
Copy link
Contributor

@bricestacey bricestacey commented Nov 8, 2022

Hoping to get a quick pair of eyes on this.

I found the fees convoluted because:

  1. we cache interest payments and the origination fee, but have a preview function for 3 more to get in real time
  2. I didn't know the best place to put the scaling value.
  3. We don't aggregate all the fees so I think sometimes we call transfer twice when we could just add the numbers together.

I think a good next step would be to extract each fee to its own function. That or have one function for fixed and one for open-term.


fundingVault.withdraw(amount, _pool);
emit FundsReclaimed(amount, _pool);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we allow this for fixed term loans if the loan is in a terminal state?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like if we allow closed-term borrowers to hit paydownPrincipal, then we should allow pool admins to call this function too? Otherwise funds would sit here I think. Alternatively we can restrict paydown to only be for open-term borrowers.

For open-term, do we need to also to use the callbackTimestamp anywhere? I thought that would gate PAs being able to reclaim funds, but I think I misunderstood it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to go ahead and stick with the predefined specs for the loans and be a little more strict, but will come up when we review money flows

I think the whole point of the callbackTimestamp is to reflect on-chain when the event happened. There really is nothing else to do with it. Funds will either be fully returned by the borrower or the PA will mark the loan in default. They can mark the default at their discretion at anytime though, so there is no restriction on it.

@@ -355,15 +414,16 @@ contract Loan is ILoan {
liquidityAsset,
IPool(_pool).firstLossVault(),
firstLossFee,
IPool(_pool).manager(),
IPool(_pool).feeVault(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fee vault should be used in this scenario as it is for a normal payment. We could optimize out the fee vault though if we wanted

principal: 500_000,
loanType: 1,
originationBps: 100,
poolFeePercentOfInterest: 100
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentionally using all the fees to be sure they're all taken care of


return amount;
}

function paydownPrincipal(uint256 amount) external onlyBorrower {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want / need a check that they don't overpay? Also, would there be any reason to limit this to open-term loans?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, also added check on overpaying as well

@@ -334,13 +353,52 @@ contract Loan is ILoan {
return payment;
}

function previewFees(uint256 amount)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future work, it might be nice to have a previewPayment function that previews the next payment + fees


fundingVault.withdraw(amount, _pool);
emit FundsReclaimed(amount, _pool);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like if we allow closed-term borrowers to hit paydownPrincipal, then we should allow pool admins to call this function too? Otherwise funds would sit here I think. Alternatively we can restrict paydown to only be for open-term borrowers.

For open-term, do we need to also to use the callbackTimestamp anywhere? I thought that would gate PAs being able to reclaim funds, but I think I misunderstood it.

@bricestacey bricestacey merged commit 499b60b into circlefin:master Nov 9, 2022
@bricestacey bricestacey deleted the open-term5 branch November 9, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants